Skip to content

Commit

Permalink
Adding NFD CR status handling
Browse files Browse the repository at this point in the history 
Status conditions are going to reflect the status
of the master/worker/gc/topology deployments/daemonsets.
No need to reflect the status of SA/Roles/ etc', since
they are going to be created during operator deployment
  • Loading branch information
@yevgeny-shnaidman
yevgeny-shnaidman committed Jun 5, 2024
1 parent 7654a84 commit 3b0266c
Show file tree
Hide file tree
Showing 7 changed files with 1,003 additions and 13 deletions.
17 changes: 13 additions & 4 deletions internal/controllers/nodefeaturediscovery_reconciler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import (
"sigs.k8s.io/node-feature-discovery-operator/internal/daemonset"
"sigs.k8s.io/node-feature-discovery-operator/internal/deployment"
"sigs.k8s.io/node-feature-discovery-operator/internal/job"
"sigs.k8s.io/node-feature-discovery-operator/internal/status"
)

const finalizerLabel = "nfd-finalizer"
Expand All @@ -51,8 +52,8 @@ type nodeFeatureDiscoveryReconciler struct {
}

func NewNodeFeatureDiscoveryReconciler(client client.Client, deploymentAPI deployment.DeploymentAPI, daemonsetAPI daemonset.DaemonsetAPI,
configmapAPI configmap.ConfigMapAPI, jobAPI job.JobAPI, scheme *runtime.Scheme) *nodeFeatureDiscoveryReconciler {
helper := newNodeFeatureDiscoveryHelperAPI(client, deploymentAPI, daemonsetAPI, configmapAPI, jobAPI, scheme)
configmapAPI configmap.ConfigMapAPI, jobAPI job.JobAPI, statusAPI status.StatusAPI, scheme *runtime.Scheme) *nodeFeatureDiscoveryReconciler {
helper := newNodeFeatureDiscoveryHelperAPI(client, deploymentAPI, daemonsetAPI, configmapAPI, jobAPI, statusAPI, scheme)
return &nodeFeatureDiscoveryReconciler{
helper: helper,
}
Expand Down Expand Up @@ -179,17 +180,19 @@ type nodeFeatureDiscoveryHelper struct {
daemonsetAPI daemonset.DaemonsetAPI
configmapAPI configmap.ConfigMapAPI
jobAPI job.JobAPI
statusAPI status.StatusAPI
scheme *runtime.Scheme
}

func newNodeFeatureDiscoveryHelperAPI(client client.Client, deploymentAPI deployment.DeploymentAPI, daemonsetAPI daemonset.DaemonsetAPI,
configmapAPI configmap.ConfigMapAPI, jobAPI job.JobAPI, scheme *runtime.Scheme) nodeFeatureDiscoveryHelperAPI {
configmapAPI configmap.ConfigMapAPI, jobAPI job.JobAPI, statusAPI status.StatusAPI, scheme *runtime.Scheme) nodeFeatureDiscoveryHelperAPI {
return &nodeFeatureDiscoveryHelper{
client: client,
deploymentAPI: deploymentAPI,
daemonsetAPI: daemonsetAPI,
configmapAPI: configmapAPI,
jobAPI: jobAPI,
statusAPI: statusAPI,
scheme: scheme,
}
}
Expand Down Expand Up @@ -345,5 +348,11 @@ func (nfdh *nodeFeatureDiscoveryHelper) handlePrune(ctx context.Context, nfdInst
}

func (nfdh *nodeFeatureDiscoveryHelper) handleStatus(ctx context.Context, nfdInstance *nfdv1.NodeFeatureDiscovery) error {
return nil
conditions := nfdh.statusAPI.GetConditions(ctx, nfdInstance)
if nfdh.statusAPI.AreConditionsEqual(nfdInstance.Status.Conditions, conditions) {
return nil
}
unmodifiedCR := nfdInstance.DeepCopy()
nfdInstance.Status.Conditions = conditions
return nfdh.client.Status().Patch(ctx, nfdInstance, client.MergeFrom(unmodifiedCR))
}
89 changes: 80 additions & 9 deletions internal/controllers/nodefeaturediscovery_reconciler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import (
"sigs.k8s.io/node-feature-discovery-operator/internal/daemonset"
"sigs.k8s.io/node-feature-discovery-operator/internal/deployment"
"sigs.k8s.io/node-feature-discovery-operator/internal/job"
"sigs.k8s.io/node-feature-discovery-operator/internal/status"
)

var _ = Describe("Reconcile", func() {
Expand Down Expand Up @@ -179,7 +180,7 @@ var _ = Describe("handleMaster", func() {
clnt = client.NewMockClient(ctrl)
mockDeployment = deployment.NewMockDeploymentAPI(ctrl)

nfdh = newNodeFeatureDiscoveryHelperAPI(clnt, mockDeployment, nil, nil, nil, scheme)
nfdh = newNodeFeatureDiscoveryHelperAPI(clnt, mockDeployment, nil, nil, nil, nil, scheme)
})

ctx := context.Background()
Expand Down Expand Up @@ -248,7 +249,7 @@ var _ = Describe("handleWorker", func() {
mockDS = daemonset.NewMockDaemonsetAPI(ctrl)
mockCM = configmap.NewMockConfigMapAPI(ctrl)

nfdh = newNodeFeatureDiscoveryHelperAPI(clnt, nil, mockDS, mockCM, nil, scheme)
nfdh = newNodeFeatureDiscoveryHelperAPI(clnt, nil, mockDS, mockCM, nil, nil, scheme)
})

ctx := context.Background()
Expand Down Expand Up @@ -344,7 +345,7 @@ var _ = Describe("handleTopology", func() {
clnt = client.NewMockClient(ctrl)
mockDS = daemonset.NewMockDaemonsetAPI(ctrl)

nfdh = newNodeFeatureDiscoveryHelperAPI(clnt, nil, mockDS, nil, nil, scheme)
nfdh = newNodeFeatureDiscoveryHelperAPI(clnt, nil, mockDS, nil, nil, nil, scheme)
})

ctx := context.Background()
Expand Down Expand Up @@ -429,7 +430,7 @@ var _ = Describe("handleGC", func() {
clnt = client.NewMockClient(ctrl)
mockDeployment = deployment.NewMockDeploymentAPI(ctrl)

nfdh = newNodeFeatureDiscoveryHelperAPI(clnt, mockDeployment, nil, nil, nil, scheme)
nfdh = newNodeFeatureDiscoveryHelperAPI(clnt, mockDeployment, nil, nil, nil, nil, scheme)
})

ctx := context.Background()
Expand Down Expand Up @@ -485,7 +486,7 @@ var _ = Describe("handleGC", func() {

var _ = Describe("hasFinalizer", func() {
It("checking return status whether finalizer set or not", func() {
nfdh := newNodeFeatureDiscoveryHelperAPI(nil, nil, nil, nil, nil, nil)
nfdh := newNodeFeatureDiscoveryHelperAPI(nil, nil, nil, nil, nil, nil, nil)

By("finalizers was empty")
nfdCR := nfdv1.NodeFeatureDiscovery{
Expand Down Expand Up @@ -529,7 +530,7 @@ var _ = Describe("setFinalizer", func() {
BeforeEach(func() {
ctrl = gomock.NewController(GinkgoT())
clnt = client.NewMockClient(ctrl)
nfdh = newNodeFeatureDiscoveryHelperAPI(clnt, nil, nil, nil, nil, nil)
nfdh = newNodeFeatureDiscoveryHelperAPI(clnt, nil, nil, nil, nil, nil, nil)
})

It("checking the return status of setFinalizer function", func() {
Expand Down Expand Up @@ -588,7 +589,7 @@ var _ = Describe("finalizeComponents", func() {
mockDS = daemonset.NewMockDaemonsetAPI(ctrl)
mockCM = configmap.NewMockConfigMapAPI(ctrl)

nfdh = newNodeFeatureDiscoveryHelperAPI(clnt, mockDeployment, mockDS, mockCM, nil, scheme)
nfdh = newNodeFeatureDiscoveryHelperAPI(clnt, mockDeployment, mockDS, mockCM, nil, nil, scheme)
})

ctx := context.Background()
Expand Down Expand Up @@ -663,7 +664,7 @@ var _ = Describe("removeFinalizer", func() {
ctrl = gomock.NewController(GinkgoT())
clnt = client.NewMockClient(ctrl)

nfdh = newNodeFeatureDiscoveryHelperAPI(clnt, nil, nil, nil, nil, scheme)
nfdh = newNodeFeatureDiscoveryHelperAPI(clnt, nil, nil, nil, nil, nil, scheme)
})

ctx := context.Background()
Expand Down Expand Up @@ -707,7 +708,7 @@ var _ = Describe("handlePrune", func() {
BeforeEach(func() {
ctrl = gomock.NewController(GinkgoT())
mockJob = job.NewMockJobAPI(ctrl)
nfdh = newNodeFeatureDiscoveryHelperAPI(nil, nil, nil, nil, mockJob, scheme)
nfdh = newNodeFeatureDiscoveryHelperAPI(nil, nil, nil, nil, mockJob, nil, scheme)
})

ctx := context.Background()
Expand Down Expand Up @@ -788,3 +789,73 @@ var _ = Describe("handlePrune", func() {
Entry("job finished, its pod failed", true, false),
)
})

var _ = Describe("handleStatus", func() {
var (
ctrl *gomock.Controller
clnt *client.MockClient
mockStatus *status.MockStatusAPI
nfdh nodeFeatureDiscoveryHelperAPI
)

BeforeEach(func() {
ctrl = gomock.NewController(GinkgoT())
clnt = client.NewMockClient(ctrl)
mockStatus = status.NewMockStatusAPI(ctrl)
nfdh = newNodeFeatureDiscoveryHelperAPI(clnt, nil, nil, nil, nil, mockStatus, scheme)
})

ctx := context.Background()
nfdCR := nfdv1.NodeFeatureDiscovery{
Status: nfdv1.NodeFeatureDiscoveryStatus{
Conditions: []metav1.Condition{},
},
}
newConditions := []metav1.Condition{}

It("conditions are equal, no status update is needed", func() {
gomock.InOrder(
mockStatus.EXPECT().GetConditions(ctx, &nfdCR).Return(newConditions),
mockStatus.EXPECT().AreConditionsEqual(newConditions, nfdCR.Status.Conditions).Return(true),
)

err := nfdh.handleStatus(ctx, &nfdCR)
Expect(err).To(BeNil())
})

It("conditions are not equal, status update is needed", func() {
statusWriter := client.NewMockStatusWriter(ctrl)
expectedNFD := nfdv1.NodeFeatureDiscovery{
Status: nfdv1.NodeFeatureDiscoveryStatus{
Conditions: newConditions,
},
}
gomock.InOrder(
mockStatus.EXPECT().GetConditions(ctx, &nfdCR).Return(newConditions),
mockStatus.EXPECT().AreConditionsEqual(newConditions, nfdCR.Status.Conditions).Return(false),
clnt.EXPECT().Status().Return(statusWriter),
statusWriter.EXPECT().Patch(ctx, &expectedNFD, gomock.Any()).Return(nil),
)

err := nfdh.handleStatus(ctx, &nfdCR)
Expect(err).To(BeNil())
})

It("conditions are not equal, status update failed", func() {
statusWriter := client.NewMockStatusWriter(ctrl)
expectedNFD := nfdv1.NodeFeatureDiscovery{
Status: nfdv1.NodeFeatureDiscoveryStatus{
Conditions: newConditions,
},
}
gomock.InOrder(
mockStatus.EXPECT().GetConditions(ctx, &nfdCR).Return(newConditions),
mockStatus.EXPECT().AreConditionsEqual(newConditions, nfdCR.Status.Conditions).Return(false),
clnt.EXPECT().Status().Return(statusWriter),
statusWriter.EXPECT().Patch(ctx, &expectedNFD, gomock.Any()).Return(fmt.Errorf("some error")),
)

err := nfdh.handleStatus(ctx, &nfdCR)
Expect(err).To(HaveOccurred())
})
})
148 changes: 148 additions & 0 deletions internal/status/mock_status.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 3b0266c

Please sign in to comment.