From fc103a60286756c0472d588a65302da30a3af8db Mon Sep 17 00:00:00 2001 From: Markus Lehtonen Date: Thu, 12 Dec 2024 20:25:40 +0200 Subject: [PATCH] Cleanup for NodeFeature API being GA Drop references to the gRPC API and don't suggest that NodeFeatureAPI could be disabled. Also update the developer guide for instructions running nfd components outside the cluster. --- cmd/nfd-master/main.go | 6 +- .../worker-daemonset/worker-daemonset.yaml | 2 - deployment/base/worker-job/worker-job.yaml | 2 - .../templates/clusterrole.yaml | 2 +- .../templates/clusterrolebinding.yaml | 2 +- .../templates/master.yaml | 2 - .../templates/nfd-gc.yaml | 2 +- .../templates/service.yaml | 20 ------ .../templates/serviceaccount.yaml | 2 +- .../templates/worker.yaml | 3 - .../helm/node-feature-discovery/values.yaml | 5 -- docs/deployment/helm.md | 1 - docs/developer-guide/index.md | 65 +++++++++---------- docs/get-started/index.md | 1 - pkg/nfd-master/nfd-master.go | 3 +- test/e2e/node_feature_discovery_test.go | 9 +-- test/e2e/utils/service.go | 45 ------------- 17 files changed, 38 insertions(+), 134 deletions(-) delete mode 100644 deployment/helm/node-feature-discovery/templates/service.yaml delete mode 100644 test/e2e/utils/service.go diff --git a/cmd/nfd-master/main.go b/cmd/nfd-master/main.go index bc702d0e7..3d2356ebd 100644 --- 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 index 955157877..f70a64388 100644 --- 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 index 6cf2c9dbf..aeba2143f 100644 --- 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 index 0b76e9afa..ea6e3e30f 100644 --- 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 index 8623de606..8331019dd 100644 --- 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 index 807cef43f..da3ca2408 100644 --- 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 index f5f7575c7..3642aa642 100644 --- 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 deleted file mode 100644 index a6e7678fb..000000000 --- a/deployment/helm/node-feature-discovery/templates/service.yaml +++ /dev/null @@ -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}} diff --git a/deployment/helm/node-feature-discovery/templates/serviceaccount.yaml b/deployment/helm/node-feature-discovery/templates/serviceaccount.yaml index 740160284..47c75a7e5 100644 --- 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 index ea5c5be62..5fd1ab744 100644 --- 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 index 9ed5e25a6..1f043d4dc 100644 --- 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 index 7cabdbd41..cdcf4ca72 100644 --- 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 index 0db4307ad..6f8d2ad61 100644 --- 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 +make build +NODE_NAME= ./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 -... +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 -... +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 index 4cff6b1ca..777177cc0 100644 --- 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 index 085afb76e..0c2625c0f 100644 --- 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 index 5e6777ac9..54c3b1458 100644 --- 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 deleted file mode 100644 index f597a7948..000000000 --- a/test/e2e/utils/service.go +++ /dev/null @@ -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{}) -}