Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nfd-master: stop creating NFD version annotations #1394

Merged
merged 1 commit into from
Oct 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions docs/get-started/introduction.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,16 +102,15 @@ NFD also annotates nodes it is running on:

| Annotation | Description
| ------------------------------------------------------------ | -----------
| [<instance>.]nfd.node.kubernetes.io/master.version | Version of the nfd-master instance running on the node. Informative use only.
| [<instance>.]nfd.node.kubernetes.io/worker.version | Version of the nfd-worker instance running on the node. Informative use only.
| [<instance>.]nfd.node.kubernetes.io/feature-labels | Comma-separated list of node labels managed by NFD. NFD uses this internally so must not be edited by users.
| [<instance>.]nfd.node.kubernetes.io/extended-resources | Comma-separated list of node extended resources managed by NFD. NFD uses this internally so must not be edited by users.

> **NOTE:** the [`-instance`](../reference/master-commandline-reference.md#instance)
> command line flag affects the annotation names

Unapplicable annotations are not created, i.e. for example master.version is
only created on nodes running nfd-master.
Unapplicable annotations are not created, i.e. for example
`nfd.node.kubernetes.io/extended-resources` is only placed if some extended
resources were created by NFD.

## Custom resources

Expand Down
1 change: 1 addition & 0 deletions pkg/apis/nfd/v1alpha1/annotations_labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ const (
FeatureLabelsAnnotation = AnnotationNs + "/feature-labels"

// MasterVersionAnnotation is the annotation that holds the version of nfd-master running on the node
// DEPRECATED: will not be used in NFD v0.15 or later.
MasterVersionAnnotation = AnnotationNs + "/master.version"

// WorkerVersionAnnotation is the annotation that holds the version of nfd-worker running on the node
Expand Down
8 changes: 1 addition & 7 deletions pkg/nfd-master/nfd-master-internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ import (
nfdinformers "sigs.k8s.io/node-feature-discovery/pkg/generated/informers/externalversions"
"sigs.k8s.io/node-feature-discovery/pkg/labeler"
"sigs.k8s.io/node-feature-discovery/pkg/utils"
"sigs.k8s.io/node-feature-discovery/pkg/version"
"sigs.k8s.io/yaml"
)

Expand Down Expand Up @@ -238,8 +237,7 @@ func TestUpdateMasterNode(t *testing.T) {
mockClient := &k8sclient.Clientset{}
mockNode := newMockNode()
Convey("When update operation succeeds", func() {
expectedPatches := []apihelper.JsonPatch{
apihelper.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.AnnotationNs+"/master.version", version.Get())}
expectedPatches := []apihelper.JsonPatch{}
mockHelper.On("GetClient").Return(mockClient, nil)
mockHelper.On("GetNode", mockClient, mockNodeName).Return(mockNode, nil)
mockHelper.On("PatchNode", mockClient, mockNodeName, mock.MatchedBy(jsonPatchMatcher(expectedPatches))).Return(nil)
Expand Down Expand Up @@ -378,7 +376,6 @@ func TestSetLabels(t *testing.T) {

Convey("When node update succeeds", func() {
expectedPatches := []apihelper.JsonPatch{
apihelper.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.WorkerVersionAnnotation, workerVer),
apihelper.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.FeatureLabelsAnnotation, strings.Join(mockLabelNames, ",")),
}
for k, v := range mockLabels {
Expand All @@ -397,7 +394,6 @@ func TestSetLabels(t *testing.T) {

Convey("When -label-whitelist is specified", func() {
expectedPatches := []apihelper.JsonPatch{
apihelper.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.WorkerVersionAnnotation, workerVer),
apihelper.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.FeatureLabelsAnnotation, "feature-2"),
apihelper.NewJsonPatch("add", "/metadata/labels", nfdv1alpha1.FeatureLabelNs+"/feature-2", mockLabels["feature-2"]),
}
Expand Down Expand Up @@ -429,7 +425,6 @@ func TestSetLabels(t *testing.T) {
"--invalid-name--": "valid-val",
"valid-name": "--invalid-val--"}
expectedPatches := []apihelper.JsonPatch{
apihelper.NewJsonPatch("add", "/metadata/annotations", instance+"."+nfdv1alpha1.WorkerVersionAnnotation, workerVer),
apihelper.NewJsonPatch("add", "/metadata/annotations",
instance+"."+nfdv1alpha1.FeatureLabelsAnnotation,
"feature-1,valid.ns/feature-2,"+vendorFeatureLabel+","+vendorProfileLabel),
Expand Down Expand Up @@ -457,7 +452,6 @@ func TestSetLabels(t *testing.T) {

Convey("When -resource-labels is specified", func() {
expectedPatches := []apihelper.JsonPatch{
apihelper.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.WorkerVersionAnnotation, workerVer),
apihelper.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.FeatureLabelsAnnotation, "feature-2"),
apihelper.NewJsonPatch("add", "/metadata/annotations", nfdv1alpha1.ExtendedResourceAnnotation, "feature-1,feature-3"),
apihelper.NewJsonPatch("add", "/metadata/labels", nfdv1alpha1.FeatureLabelNs+"/feature-2", mockLabels["feature-2"]),
Expand Down
24 changes: 11 additions & 13 deletions pkg/nfd-master/nfd-master.go
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,10 @@ func (m *nfdMaster) prune() error {
return nil
}

// Advertise NFD master information
// Update annotations on the node where nfd-master is running. Currently the
// only function is to remove the deprecated
// "nfd.node.kubernetes.io/master.version" annotation, if it exists.
// TODO: Drop when nfdv1alpha1.MasterVersionAnnotation is removed.
func (m *nfdMaster) updateMasterNode() error {
cli, err := m.apihelper.GetClient()
if err != nil {
Expand All @@ -491,9 +494,9 @@ func (m *nfdMaster) updateMasterNode() error {
}

// Advertise NFD version as an annotation
p := createPatches(nil,
p := createPatches([]string{m.instanceAnnotation(nfdv1alpha1.MasterVersionAnnotation)},
node.Annotations,
Annotations{m.instanceAnnotation(nfdv1alpha1.MasterVersionAnnotation): version.Get()},
nil,
"/metadata/annotations")
err = m.apihelper.PatchNode(cli, node.Name, p)
if err != nil {
Expand Down Expand Up @@ -680,8 +683,7 @@ func (m *nfdMaster) SetLabels(c context.Context, r *pb.SetLabelsRequest) (*pb.Se
return &pb.SetLabelsReply{}, err
}

// Advertise NFD worker version as an annotation
annotations := Annotations{m.instanceAnnotation(nfdv1alpha1.WorkerVersionAnnotation): r.NfdVersion}
annotations := Annotations{}

// Create labels et al
if err := m.refreshNodeFeatures(cli, r.NodeName, annotations, r.GetLabels(), r.GetFeatures()); err != nil {
Expand Down Expand Up @@ -761,13 +763,6 @@ func (m *nfdMaster) nfdAPIUpdateOneNode(nodeName string) error {
}

klog.V(4).InfoS("merged nodeFeatureSpecs", "newNodeFeatureSpec", utils.DelayedDumper(features))

if objs[0].Namespace == m.namespace && objs[0].Name == nodeName {
// This is the one created by nfd-worker
if v := objs[0].Annotations[nfdv1alpha1.WorkerVersionAnnotation]; v != "" {
annotations[nfdv1alpha1.WorkerVersionAnnotation] = v
}
}
}

// Update node labels et al. This may also mean removing all NFD-owned
Expand Down Expand Up @@ -1067,7 +1062,10 @@ func (m *nfdMaster) updateNodeObject(cli *kubernetes.Clientset, nodeName string,
createPatches(
[]string{
m.instanceAnnotation(nfdv1alpha1.FeatureLabelsAnnotation),
m.instanceAnnotation(nfdv1alpha1.ExtendedResourceAnnotation)},
m.instanceAnnotation(nfdv1alpha1.ExtendedResourceAnnotation),
// Clean up deprecated/stale nfd version annotations
m.instanceAnnotation(nfdv1alpha1.MasterVersionAnnotation),
m.instanceAnnotation(nfdv1alpha1.WorkerVersionAnnotation)},
node.Annotations,
annotations,
"/metadata/annotations")...)
Expand Down
2 changes: 0 additions & 2 deletions test/e2e/e2e-test-config.example.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,6 @@ defaultFeatures:
- "feature.node.kubernetes.io/system-os_release.VERSION_ID.major"
- "feature.node.kubernetes.io/system-os_release.VERSION_ID.minor"
annotationWhitelist:
- "nfd.node.kubernetes.io/master.version"
- "nfd.node.kubernetes.io/worker.version"
- "nfd.node.kubernetes.io/feature-labels"
nodes:
- name: name-of-this-item # name is purely informational
Expand Down
10 changes: 0 additions & 10 deletions test/e2e/node_feature_discovery_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,16 +249,6 @@ var _ = SIGDescribe("NFD master and worker", func() {

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("Verifying the node where nfd-master is running")
// Get updated masterPod object (we want to know where it was scheduled)
masterPod, err = f.ClientSet.CoreV1().Pods(f.Namespace.Name).Get(ctx, masterPod.Name, metav1.GetOptions{})
Expect(err).NotTo(HaveOccurred())
// Node running nfd-master should have master version annotation
masterPodNode, err := f.ClientSet.CoreV1().Nodes().Get(ctx, masterPod.Spec.NodeName, metav1.GetOptions{})
Expect(err).NotTo(HaveOccurred())
Expect(masterPodNode.Annotations).To(HaveKey(nfdv1alpha1.AnnotationNs + "/master.version"))

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())
})
Expand Down