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

k8s-ci-robot · web-flow · commit 3e87c97ac29e · 2024-12-13T15:14:26.000+01:00
Cleanup for NodeFeature API being GA
diff --git a/cmd/nfd-master/main.go b/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",
-		"Specify the NFD API controller resync period."+
-			"It does not have effect when the NodeFeature API has been disabled (with -feature-gates NodeFeatureAPI=false).")
+	flagset.Var(overrides.ResyncPeriod, "resync-period", "Specify the NFD API controller resync period.")
 	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
 }
diff --git a/deployment/base/worker-daemonset/worker-daemonset.yaml b/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
diff --git a/deployment/base/worker-job/worker-job.yaml b/deployment/base/worker-job/worker-job.yaml
@@ -33,5 +33,3 @@ spec:
             - "nfd-worker"
           args:
             - "-oneshot"
-            - "-server=nfd-master:8080"
-
diff --git a/deployment/helm/node-feature-discovery/templates/clusterrole.yaml b/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
diff --git a/deployment/helm/node-feature-discovery/templates/clusterrolebinding.yaml b/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
diff --git a/deployment/helm/node-feature-discovery/templates/master.yaml b/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" }}
diff --git a/deployment/helm/node-feature-discovery/templates/nfd-gc.yaml b/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:
diff --git a/deployment/helm/node-feature-discovery/templates/service.yaml b/deployment/helm/node-feature-discovery/templates/service.yaml
diff --git a/deployment/helm/node-feature-discovery/templates/serviceaccount.yaml b/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
diff --git a/deployment/helm/node-feature-discovery/templates/worker.yaml b/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 }}"
diff --git a/deployment/helm/node-feature-discovery/values.yaml b/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
diff --git a/docs/deployment/helm.md b/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                                                                                                                                                                                                                       |
diff --git a/docs/developer-guide/index.md b/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
-Kubernetes API is not available. Thus, it is recommended to use `-no-publish`.
+For development and debugging it is possible to run nfd-master as a stand-alone
+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 }}
-$ docker run --rm --name=nfd-test ${NFD_CONTAINER_IMAGE} nfd-master -no-publish -crd-controller=false -feature-gates NodeFeatureAPI=false
-2019/02/01 14:48:21 Node Feature Discovery Master <NFD_VERSION>
+make build
+NODE_NAME=<EXISTING_NODE> ./nfd-master -no-publish -kubeconfig ~/.kube/config
 ```
 
 ### NFD-Worker
 
-To run nfd-worker as a "stand-alone" container you need to run it in the same
-network namespace as the nfd-master container:
+For development and debugging it is possible to run nfd-worker as a stand-alone
+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
-2019/02/01 14:48:56 Node Feature Discovery Worker <NFD_VERSION>
-...
+make build
+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,
-pass the `-no-publish` flag to nfd-worker.
-
-> **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.
+> **NOTE:** Running nfd-worker locally this way discovers and publishes
+> features of the local development system you're running nfd-worker on.
 
 ### NFD-Topology-Updater
 
-To run nfd-topology-updater as a "stand-alone" container
-you need to run it in with the `-no-publish` flag to disable communication to
-the Kubernetes apiserver.
+For development and debugging it is possible to run nfd-topology-updater as a
+stand-alone binary outside the cluster. However, it requires access to the
+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
-2019/02/01 14:48:56 Node Feature Discovery Topology Updater <NFD_VERSION>
-...
+make build
+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
diff --git a/docs/get-started/index.md b/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
 
diff --git a/pkg/nfd-master/nfd-master.go b/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
-// arriving through the gRPC API.
+// labels and ERs, i.e. adds the possibly missing default namespace for labels.
 func (m *nfdMaster) filterFeatureLabels(labels Labels, features *nfdv1alpha1.Features) Labels {
 	outLabels := Labels{}
 	for name, value := range labels {
diff --git a/test/e2e/node_feature_discovery_test.go b/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) {
diff --git a/test/e2e/utils/service.go b/test/e2e/utils/service.go

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-{{- if and .Values.gc.enable (or .Values.featureGates.NodeFeatureAPI .Values.topologyUpdater.enable) -}}`
	`1`	`+{{- if and .Values.gc.enable -}}`
`2`	`2`	`apiVersion: apps/v1`
`3`	`3`	`kind: Deployment`
`4`	`4`	`metadata:`