Merge pull request #1976 from marquiz/devel/grpc-api-cleanup

Cleanup for NodeFeature API being GA

cmd/nfd-master/main.go

@@ -142,11 +142,9 @@ func initFlags(flagset *flag.FlagSet) (*master.Args, *master.ConfigOverrideArgs)
 		"Do not publish feature labels")
 	flagset.Var(overrides.DenyLabelNs, "deny-label-ns",
 		"Comma separated list of denied label namespaces")
-	flagset.Var(overrides.ResyncPeriod, "resync-period",
+	flagset.Var(overrides.ResyncPeriod, "resync-period", "Specify the NFD API controller resync period.")
-		"Specify the NFD API controller resync period."+
-			"It does not have effect when the NodeFeature API has been disabled (with -feature-gates NodeFeatureAPI=false).")
 	overrides.NfdApiParallelism = flagset.Int("nfd-api-parallelism", 10, "Defines the maximum number of goroutines responsible of updating nodes. "+
-		"Can be used for the throttling mechanism. It does not have effect if NodeFeatureAPI feature gate is disabled.")
+		"Can be used for the throttling mechanism.")
 
 	return args, overrides
 }

deployment/base/worker-daemonset/worker-daemonset.yaml

@@ -39,8 +39,6 @@ spec:
             requests:
               cpu: 5m
               memory: 64Mi
-          args:
-            - "-server=nfd-master:8080"
           ports:
             - name: metrics
               containerPort: 8081

deployment/base/worker-job/worker-job.yaml

@@ -33,5 +33,3 @@ spec:
             - "nfd-worker"
           args:
             - "-oneshot"
-            - "-server=nfd-master:8080"
-

deployment/helm/node-feature-discovery/templates/clusterrole.yaml

@@ -101,7 +101,7 @@ rules:
   - update
 {{- end }}
 
-{{- if and .Values.gc.enable .Values.gc.rbac.create (or .Values.featureGates.NodeFeatureAPI .Values.topologyUpdater.enable) }}
+{{- if and .Values.gc.enable .Values.gc.rbac.create }}
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRole

deployment/helm/node-feature-discovery/templates/clusterrolebinding.yaml

@@ -33,7 +33,7 @@ subjects:
   namespace: {{ include "node-feature-discovery.namespace" .  }}
 {{- end }}
 
-{{- if and .Values.gc.enable .Values.gc.rbac.create (or .Values.featureGates.NodeFeatureAPI  .Values.topologyUpdater.enable) }}
+{{- if and .Values.gc.enable .Values.gc.rbac.create }}
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRoleBinding

deployment/helm/node-feature-discovery/templates/master.yaml

@@ -96,8 +96,6 @@ spec:
             successThreshold: {{ . }}
           {{- end }}
           ports:
-          - containerPort: {{ .Values.master.port | default "8080" }}
-            name: grpc
           - containerPort: {{ .Values.master.metricsPort | default "8081" }}
             name: metrics
           - containerPort: {{ .Values.master.healthPort | default "8082" }}

deployment/helm/node-feature-discovery/templates/nfd-gc.yaml

@@ -1,4 +1,4 @@
-{{- if and .Values.gc.enable (or .Values.featureGates.NodeFeatureAPI .Values.topologyUpdater.enable) -}}
+{{- if and .Values.gc.enable -}}
 apiVersion: apps/v1
 kind: Deployment
 metadata:

deployment/helm/node-feature-discovery/templates/service.yaml

@@ -1,20 +0,0 @@
-{{- if and (not .Values.featureGates.NodeFeatureAPI) .Values.master.enable }}
-apiVersion: v1
-kind: Service
-metadata:
-  name: {{ include "node-feature-discovery.fullname" . }}-master
-  namespace: {{ include "node-feature-discovery.namespace" . }}
-  labels:
-    {{- include "node-feature-discovery.labels" . | nindent 4 }}
-    role: master
-spec:
-  type: {{ .Values.master.service.type }}
-  ports:
-    - port: {{ .Values.master.service.port | default "8080" }}
-      targetPort: grpc
-      protocol: TCP
-      name: grpc
-  selector:
-    {{- include "node-feature-discovery.selectorLabels" . | nindent 4 }}
-    role: master
-{{- end}}

deployment/helm/node-feature-discovery/templates/serviceaccount.yaml

@@ -27,7 +27,7 @@ metadata:
   {{- end }}
 {{- end }}
 
-{{- if and .Values.gc.enable .Values.gc.serviceAccount.create (or .Values.featureGates.NodeFeatureAPI .Values.topologyUpdater.enable) }}
+{{- if and .Values.gc.enable .Values.gc.serviceAccount.create }}
 ---
 apiVersion: v1
 kind: ServiceAccount

deployment/helm/node-feature-discovery/templates/worker.yaml

@@ -100,9 +100,6 @@ spec:
         command:
         - "nfd-worker"
         args:
-        {{- if not .Values.featureGates.NodeFeatureAPI }}
-        - "-server={{ include "node-feature-discovery.fullname" . }}-master:{{ .Values.master.service.port }}"
-        {{- end }}
         # Go over featureGate and add the feature-gate flag
         {{- range $key, $value := .Values.featureGates }}
         - "-feature-gates={{ $key }}={{ $value }}"

deployment/helm/node-feature-discovery/values.yaml

@@ -11,7 +11,6 @@ fullnameOverride: ""
 namespaceOverride: ""
 
 featureGates:
-  NodeFeatureAPI: true
   NodeFeatureGroupAPI: false
 
 priorityClassName: ""
@@ -106,10 +105,6 @@ master:
   rbac:
     create: true
 
-  service:
-    type: ClusterIP
-    port: 8080
-
   resources:
     limits:
       memory: 4Gi

docs/deployment/helm.md

@@ -158,7 +158,6 @@ Chart parameters are available.
 | `imagePullSecrets`                                  | array  | []                                                  | ImagePullSecrets is an optional list of references to secrets in the same namespace to use for pulling any of the images used by this PodSpec. [More info](https://kubernetes.io/docs/concepts/containers/images#specifying-imagepullsecrets-on-a-pod).                             |
 | `nameOverride`                                      | string |                                                     | Override the name of the chart                                                                                                                                                                                                                                                      |
 | `fullnameOverride`                                  | string |                                                     | Override a default fully qualified app name                                                                                                                                                                                                                                         |
-| `featureGates.NodeFeatureAPI`                       | bool   | true                                                | Enable the [NodeFeature](../usage/custom-resources.md#nodefeature) CRD API for communicating node features. This will automatically disable the gRPC communication.                                                                                                                 |
 | `featureGates.NodeFeatureGroupAPI`                  | bool   | false                                               | Enable the [NodeFeatureGroup](../usage/custom-resources.md#nodefeaturegroup) CRD API.                                                                                                                                                                                               |
 | `featureGates.DisableAutoPrefix`                    | bool   | false                                               | Enable [DisableAutoPrefix](../reference/feature-gates.md#disableautoprefix) feature gate. Disables automatic prefixing of unprefixed labels, annotations and extended resources.                                                                                                    |
 | `prometheus.enable`                                 | bool   | false                                               | Specifies whether to expose metrics using prometheus operator                                                                                                                                                                                                                       |

docs/developer-guide/index.md

@@ -173,58 +173,53 @@ e2e-tests:
 
 ### NFD-Master
 
-When running as a standalone container labeling is expected to fail because
+For development and debugging it is possible to run nfd-master as a stand-alone
-Kubernetes API is not available. Thus, it is recommended to use `-no-publish`.
+binary outside the cluster. The `-no-publish` flag can be used to prevent
+nfd-master making changes to the nodes. If `-no-publish` is not set, nfd-master
+also requires the `NODE_NAME` environment variable to be set for cleaning up
+stale annotations.
 
 ```bash
-$ export NFD_CONTAINER_IMAGE={{ site.container_image }}
+make build
-$ docker run --rm --name=nfd-test ${NFD_CONTAINER_IMAGE} nfd-master -no-publish -crd-controller=false -feature-gates NodeFeatureAPI=false
+NODE_NAME=<EXISTING_NODE> ./nfd-master -no-publish -kubeconfig ~/.kube/config
-2019/02/01 14:48:21 Node Feature Discovery Master <NFD_VERSION>
 ```
 
 ### NFD-Worker
 
-To run nfd-worker as a "stand-alone" container you need to run it in the same
+For development and debugging it is possible to run nfd-worker as a stand-alone
-network namespace as the nfd-master container:
+binary outside the cluster. The `-no-publish` flag can be used to prevent
+nfd-worker from creating NodeFeature objects in the target cluster. If the
+`-no-publish` is not set, nfd-worker also requires the `NODE_NAME` and
+`KUBERNETES_NAMESPACE` environment variables to be defined to create the
+NodeFeature object in the target cluster.
 
 ```bash
-$ docker run --rm --network=container:nfd-test ${NFD_CONTAINER_IMAGE} nfd-worker -feature-gates NodeFeatureAPI=false
+make build
-2019/02/01 14:48:56 Node Feature Discovery Worker <NFD_VERSION>
+KUBERNETES_NAMESPACE=default NODE_NAME=nonexistent-node ./bin/nfd-worker -kubeconfig ~/.kube/config
-...
 ```
 
-If you just want to try out feature discovery without connecting to nfd-master,
+> **NOTE:** Running nfd-worker locally this way discovers and publishes
-pass the `-no-publish` flag to nfd-worker.
+> features of the local development system you're running nfd-worker on.
-
-> **NOTE:** Some feature sources need certain directories and/or files from the
-> host mounted inside the NFD container. Thus, you need to provide Docker with
-> the correct `--volume` options for them to work correctly when run
-> stand-alone directly with `docker run`. See
-> the [default deployment](https://github.com/kubernetes-sigs/node-feature-discovery/blob/{{site.release}}/deployment/components/common/worker-mounts.yaml)
-> for up-to-date information about the required volume mounts.
 
 ### NFD-Topology-Updater
 
-To run nfd-topology-updater as a "stand-alone" container
+For development and debugging it is possible to run nfd-topology-updater as a
-you need to run it in with the `-no-publish` flag to disable communication to
+stand-alone binary outside the cluster. However, it requires access to the
-the Kubernetes apiserver.
+kubelet's local pod-resources socket and the kubelet http api so in practice it
+needs to be run on a host acting as a Kubernetes node and thus running
+kubelet. Running kubelet with `--read-only-port=10255` (or `readOnlyPort:
+10255` in config) makes it possible to connect to kubelet without auth-token
+(never do this in a production cluster). Also, the `-no-publish` flag can be
+used to prevent nfd-topology-updater from creating NodeResourceTopology objects
+in the target cluster. If the `-no-publish` is not set, nfd-topology-updater
+also requires the `NODE_NAME` and `KUBERNETES_NAMESPACE` environment variables
+to be defined.
 
 ```bash
-$ docker run --rm ${NFD_CONTAINER_IMAGE} nfd-topology-updater -no-publish
+make build
-2019/02/01 14:48:56 Node Feature Discovery Topology Updater <NFD_VERSION>
+KUBERNETES_NAMESPACE=default NODE_NAME=nonexistent-node ./bin/nfd-topology-updater -kubeconfig ~/.kube/config -kubelet-config-uri http://127.0.0.1:10255
-...
 ```
 
-If you just want to try out resource topology discovery without connecting to
-the Kubernetes API, pass the `-no-publish` flag to nfd-topology-updater.
-
-> **NOTE:** NFD topology updater needs certain directories and/or files from
-> the host mounted inside the NFD container. Thus, you need to provide Docker
-> with the correct `--volume` options for them to work correctly when
-> run stand-alone directly with `docker run`. See
-> the [template spec](https://github.com/kubernetes-sigs/node-feature-discovery/blob/{{site.release}}/deployment/components/topology-updater/topologyupdater-mounts.yaml)
-> for up-to-date information about the required volume mounts.
-
 ## Running with Tilt
 
 Another option for building NFD locally is via Tilt tool, which can build container

docs/get-started/index.md

@@ -26,7 +26,6 @@ $ kubectl apply -k https://github.com/kubernetes-sigs/node-feature-discovery/dep
   clusterrole.rbac.authorization.k8s.io/nfd-master created
   clusterrolebinding.rbac.authorization.k8s.io/nfd-master created
   configmap/nfd-worker-conf created
-  service/nfd-master created
   deployment.apps/nfd-master created
   daemonset.apps/nfd-worker created
 

pkg/nfd-master/nfd-master.go

@@ -521,8 +521,7 @@ func (m *nfdMaster) updateMasterNode() error {
 
 // Filter labels by namespace and name whitelist, and, turn selected labels
 // into extended resources. This function also handles proper namespacing of
-// labels and ERs, i.e. adds the possibly missing default namespace for labels
+// labels and ERs, i.e. adds the possibly missing default namespace for labels.
-// arriving through the gRPC API.
 func (m *nfdMaster) filterFeatureLabels(labels Labels, features *nfdv1alpha1.Features) Labels {
 	outLabels := Labels{}
 	for name, value := range labels {

test/e2e/node_feature_discovery_test.go

@@ -38,7 +38,6 @@ import (
 	clientset "k8s.io/client-go/kubernetes"
 	taintutils "k8s.io/kubernetes/pkg/util/taints"
 	"k8s.io/kubernetes/test/e2e/framework"
-	e2enetwork "k8s.io/kubernetes/test/e2e/framework/network"
 	e2epod "k8s.io/kubernetes/test/e2e/framework/pod"
 	admissionapi "k8s.io/pod-security-admission/api"
 
@@ -242,19 +241,13 @@ var _ = NFDDescribe(Label("nfd-master"), func() {
 			cleanupNode(ctx, f.ClientSet)
 
 			// Launch nfd-master
-			By("Creating nfd master pod and nfd-master service")
+			By("Creating nfd master pod")
 			podSpecOpts := append(extraMasterPodSpecOpts, testpod.SpecWithContainerImage(dockerImage()))
 
 			masterPod := e2epod.NewPodClient(f).CreateSync(ctx, testpod.NFDMaster(podSpecOpts...))
 
-			// Create nfd-master service
-			nfdSvc, err := testutils.CreateService(ctx, f.ClientSet, f.Namespace.Name)
-			Expect(err).NotTo(HaveOccurred())
-
 			By("Waiting for the nfd-master pod to be running")
 			Expect(e2epod.WaitTimeoutForPodRunningInNamespace(ctx, f.ClientSet, masterPod.Name, masterPod.Namespace, time.Minute)).NotTo(HaveOccurred())
-			By("Waiting for the nfd-master service to be up")
-			Expect(e2enetwork.WaitForService(ctx, f.ClientSet, f.Namespace.Name, nfdSvc.Name, true, time.Second, 10*time.Second)).NotTo(HaveOccurred())
 		})
 
 		AfterEach(func(ctx context.Context) {

test/e2e/utils/service.go

@@ -1,45 +0,0 @@
-/*
-Copyright 2018-2022 The Kubernetes Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package utils
-
-import (
-	"context"
-
-	corev1 "k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	clientset "k8s.io/client-go/kubernetes"
-)
-
-// CreateService creates nfd-master Service
-func CreateService(ctx context.Context, cs clientset.Interface, ns string) (*corev1.Service, error) {
-	svc := &corev1.Service{
-		ObjectMeta: metav1.ObjectMeta{
-			Name: "nfd-master-e2e",
-		},
-		Spec: corev1.ServiceSpec{
-			Selector: map[string]string{"name": "nfd-master-e2e"},
-			Ports: []corev1.ServicePort{
-				{
-					Protocol: corev1.ProtocolTCP,
-					Port:     8080,
-				},
-			},
-			Type: corev1.ServiceTypeClusterIP,
-		},
-	}
-	return cs.CoreV1().Services(ns).Create(ctx, svc, metav1.CreateOptions{})
-}