Skip to content

Commit c77da66

Browse files
heyvister1heyvister1
committed
setting nodestate annotation with 'sriovnetwork.openshift.io/use-external-drainer=true' in case exteranl drainer is enabled
The motivation is for external drainer verification, that SRIOV operator is set with external drainer Signed-off-by: Ido Heyvi <iheyvi@nvidia.com>
1 parent f094d65 commit c77da66

File tree

7 files changed

+114
-38
lines changed

7 files changed

+114
-38
lines changed

‎bindata/manifests/daemon/daemonset.yaml‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -186,14 +186,10 @@ spec:
186186
value: "{{.ClusterType}}"
187187
- name: DEV_MODE
188188
value: "{{.DevMode}}"
189-
{{- if .UseExternalDrainer }}
189+
{{- if .UseExternalDrainer }}
190190
- name: USE_EXTERNAL_DRAINER
191191
value: "{{.UseExternalDrainer}}"
192-
{{- end }}
193-
{{- range $key, $value := .ConfigDaemonEnvVars }}
194-
- name: {{ $key }}
195-
value: "{{ $value }}"
196-
{{- end }}
192+
{{- end }}
197193
resources:
198194
requests:
199195
cpu: 100m

‎controllers/drain_controller_test.go‎

Lines changed: 21 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ var _ = Describe("Drain Controller", Ordered, func() {
138138
})
139139

140140
It("should drain single node on drain require", func(ctx context.Context) {
141-
node, nodeState := createNode(ctx, "node1", nil)
141+
node, nodeState := createNode(ctx, "node1", false)
142142

143143
simulateDaemonSetAnnotation(node, constants.DrainRequired)
144144

@@ -152,7 +152,7 @@ var _ = Describe("Drain Controller", Ordered, func() {
152152
})
153153

154154
It("should not drain on reboot for single node", func(ctx context.Context) {
155-
node, nodeState := createNode(ctx, "node1", nil)
155+
node, nodeState := createNode(ctx, "node1", false)
156156

157157
simulateDaemonSetAnnotation(node, constants.RebootRequired)
158158

@@ -165,8 +165,8 @@ var _ = Describe("Drain Controller", Ordered, func() {
165165
})
166166

167167
It("should drain on reboot for multiple node", func(ctx context.Context) {
168-
node, nodeState := createNode(ctx, "node1", nil)
169-
createNode(ctx, "node2", nil)
168+
node, nodeState := createNode(ctx, "node1", false)
169+
createNode(ctx, "node2", false)
170170

171171
simulateDaemonSetAnnotation(node, constants.RebootRequired)
172172

@@ -182,9 +182,9 @@ var _ = Describe("Drain Controller", Ordered, func() {
182182
Context("when there are multiple nodes", func() {
183183

184184
It("should drain nodes serially with default pool selector", func(ctx context.Context) {
185-
node1, nodeState1 := createNode(ctx, "node1", nil)
186-
node2, nodeState2 := createNode(ctx, "node2", nil)
187-
node3, nodeState3 := createNode(ctx, "node3", nil)
185+
node1, nodeState1 := createNode(ctx, "node1", false)
186+
node2, nodeState2 := createNode(ctx, "node2", false)
187+
node3, nodeState3 := createNode(ctx, "node3", false)
188188

189189
// Two nodes require to drain at the same time
190190
simulateDaemonSetAnnotation(node1, constants.DrainRequired)
@@ -222,9 +222,9 @@ var _ = Describe("Drain Controller", Ordered, func() {
222222
})
223223

224224
It("should drain nodes in parallel with a custom pool selector", func(ctx context.Context) {
225-
node1, nodeState1 := createNode(ctx, "node1", nil)
226-
node2, nodeState2 := createNode(ctx, "node2", nil)
227-
node3, nodeState3 := createNode(ctx, "node3", nil)
225+
node1, nodeState1 := createNode(ctx, "node1", false)
226+
node2, nodeState2 := createNode(ctx, "node2", false)
227+
node3, nodeState3 := createNode(ctx, "node3", false)
228228

229229
maxun := intstr.Parse("2")
230230
poolConfig := &sriovnetworkv1.SriovNetworkPoolConfig{}
@@ -273,9 +273,9 @@ var _ = Describe("Drain Controller", Ordered, func() {
273273
})
274274

275275
It("should drain nodes in parallel with a custom pool selector and honor MaxUnavailable", func(ctx context.Context) {
276-
node1, nodeState1 := createNode(ctx, "node1", nil)
277-
node2, nodeState2 := createNode(ctx, "node2", nil)
278-
node3, nodeState3 := createNode(ctx, "node3", nil)
276+
node1, nodeState1 := createNode(ctx, "node1", false)
277+
node2, nodeState2 := createNode(ctx, "node2", false)
278+
node3, nodeState3 := createNode(ctx, "node3", false)
279279

280280
maxun := intstr.Parse("2")
281281
poolConfig := &sriovnetworkv1.SriovNetworkPoolConfig{}
@@ -298,9 +298,9 @@ var _ = Describe("Drain Controller", Ordered, func() {
298298
})
299299

300300
It("should drain all nodes in parallel with a custom pool using nil in max unavailable", func(ctx context.Context) {
301-
node1, nodeState1 := createNode(ctx, "node1", nil)
302-
node2, nodeState2 := createNode(ctx, "node2", nil)
303-
node3, nodeState3 := createNode(ctx, "node3", nil)
301+
node1, nodeState1 := createNode(ctx, "node1", false)
302+
node2, nodeState2 := createNode(ctx, "node2", false)
303+
node3, nodeState3 := createNode(ctx, "node3", false)
304304

305305
poolConfig := &sriovnetworkv1.SriovNetworkPoolConfig{}
306306
poolConfig.SetNamespace(testNamespace)
@@ -326,7 +326,7 @@ var _ = Describe("Drain Controller", Ordered, func() {
326326
})
327327

328328
It("should drain in parallel nodes from two different pools, one custom and one default", func() {
329-
node1, nodeState1 := createNode(ctx, "node1", nil)
329+
node1, nodeState1 := createNode(ctx, "node1", false)
330330
node2, nodeState2 := createNodeWithLabel(ctx, "node2", "pool")
331331
createPodOnNode(ctx, "test-node-2", "node2")
332332

@@ -349,7 +349,7 @@ var _ = Describe("Drain Controller", Ordered, func() {
349349
})
350350

351351
It("should select all the nodes to drain in parallel when the selector is empty", func() {
352-
node1, nodeState1 := createNode(ctx, "node3", nil)
352+
node1, nodeState1 := createNode(ctx, "node3", false)
353353
node2, nodeState2 := createNodeWithLabel(ctx, "node4", "pool")
354354
createPodOnNode(ctx, "test-empty-1", "node3")
355355
createPodOnNode(ctx, "test-empty-2", "node4")
@@ -431,8 +431,7 @@ func simulateDaemonSetAnnotation(node *corev1.Node, drainAnnotationValue string)
431431
ToNot(HaveOccurred())
432432
}
433433

434-
func createNode(ctx context.Context, nodeName string,
435-
additionalAnnotations map[string]string) (*corev1.Node, *sriovnetworkv1.SriovNetworkNodeState) {
434+
func createNode(ctx context.Context, nodeName string, useExternalDrainer bool) (*corev1.Node, *sriovnetworkv1.SriovNetworkNodeState) {
436435
node := corev1.Node{
437436
ObjectMeta: metav1.ObjectMeta{
438437
Name: nodeName,
@@ -456,8 +455,8 @@ func createNode(ctx context.Context, nodeName string,
456455
},
457456
}
458457

459-
for key, value := range additionalAnnotations {
460-
nodeState.Annotations[key] = value
458+
if useExternalDrainer {
459+
nodeState.Annotations[constants.NodeStateExternalDrainerAnnotation] = "true"
461460
}
462461

463462
Expect(k8sClient.Create(ctx, &node)).ToNot(HaveOccurred())

‎deployment/sriov-network-operator-chart/templates/operator.yaml‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,10 @@ spec:
8787
value: {{ .Values.operator.metricsExporter.certificates.secretName }}
8888
- name: METRICS_EXPORTER_KUBE_RBAC_PROXY_IMAGE
8989
value: {{ .Values.images.metricsExporterKubeRbacProxy }}
90-
- name: USE_EXTERNAL_DRAINER
91-
value: {{ .Values.operator.externalDrainer.enabled | quote }}
90+
{{- if .Values.operator.externalDrainer.enabled }}
91+
- name: USE_EXTERNAL_DRAINER
92+
value: {{ .Values.operator.externalDrainer.enabled | quote }}
93+
{{- end }}
9294
{{- if .Values.operator.metricsExporter.prometheusOperator.enabled }}
9395
- name: METRICS_EXPORTER_PROMETHEUS_OPERATOR_ENABLED
9496
value: {{ .Values.operator.metricsExporter.prometheusOperator.enabled | quote}}

‎main.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -274,8 +274,7 @@ func main() {
274274
func setupDrainController(mgr ctrl.Manager, restConfig *rest.Config,
275275
platformsHelper platforms.Interface, scheme *runtime.Scheme) error {
276276
if vars.UseExternalDrainer {
277-
setupLog.Info("internal drain controller is disabled, draining will be done externally by the maintenance operator")
278-
return nil
277+
setupLog.Info("'UseExternalDrainer' is set, draining will be done externally")
279278
}
280279

281280
// we need a client that doesn't use the local cache for the objects

‎pkg/consts/constants.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,6 @@ const (
9797
NodeStateDrainAnnotation = "sriovnetwork.openshift.io/desired-state"
9898
NodeStateDrainAnnotationCurrent = "sriovnetwork.openshift.io/current-state"
9999
NodeStateExternalDrainerAnnotation = "sriovnetwork.openshift.io/use-external-drainer"
100-
DesiredMachineConfigAnnotation = "machineconfiguration.openshift.io/desiredConfig"
101100
DrainIdle = "Idle"
102101
DrainRequired = "Drain_Required"
103102
RebootRequired = "Reboot_Required"

‎pkg/daemon/daemon.go‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,16 @@ func (dn *NodeReconciler) apply(ctx context.Context, desiredNodeState *sriovnetw
445445
return ctrl.Result{}, err
446446
}
447447

448+
// remove external drainer nodestate annotation
449+
err = utils.AnnotateObject(ctx, desiredNodeState,
450+
consts.NodeStateExternalDrainerAnnotation, "", dn.client)
451+
if err != nil {
452+
if !errors.IsNotFound(err) {
453+
reqLogger.Error(err, "failed to remove node external drainer annotation")
454+
return ctrl.Result{}, err
455+
}
456+
}
457+
448458
reqLogger.Info("sync succeeded")
449459
syncStatus := consts.SyncStatusSucceeded
450460
lastSyncError := ""
@@ -600,6 +610,16 @@ func (dn *NodeReconciler) handleDrain(ctx context.Context, desiredNodeState *sri
600610
return false, nil
601611
}
602612

613+
// add external drainer nodestate annotation if flag is enabled
614+
if vars.UseExternalDrainer {
615+
err := utils.AnnotateObject(ctx, desiredNodeState,
616+
consts.NodeStateExternalDrainerAnnotation, "true", dn.client)
617+
if err != nil {
618+
funcLog.Error(err, "failed to add node external drainer annotation")
619+
return false, err
620+
}
621+
}
622+
603623
// annotate both node and node state with drain or reboot
604624
annotation := consts.DrainRequired
605625
if reqReboot {

‎pkg/daemon/daemon_test.go‎

Lines changed: 66 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,9 @@ var (
6868
)
6969

7070
const (
71-
waitTime = 30 * time.Minute
72-
retryTime = 5 * time.Second
73-
nodeName = "node1"
74-
devicePluginPodName = "sriov-device-plugin-test"
71+
waitTime = 30 * time.Minute
72+
retryTime = 5 * time.Second
73+
nodeName = "node1"
7574
)
7675

7776
// newSriovDiscoverReturn creates a new sriovDiscoverReturn instance.
@@ -247,7 +246,8 @@ var _ = Describe("Daemon Controller", Ordered, func() {
247246
vars.FeatureGate = featureGates
248247
daemonReconciler = createDaemon(platformMock, featureGates, []string{})
249248
startDaemon(daemonReconciler)
250-
createNode(nodeName)
249+
250+
_, nodeState = createNode(nodeName)
251251
})
252252

253253
AfterAll(func() {
@@ -669,6 +669,67 @@ var _ = Describe("Daemon Controller", Ordered, func() {
669669
g.Expect(devicePluginPod.Annotations).ToNot(HaveKey(constants.DevicePluginWaitConfigAnnotation))
670670
}, waitTime, retryTime).Should(Succeed())
671671
})
672+
673+
It("Should apply external drainer annotation when useExternalDrainer is true", func(ctx context.Context) {
674+
DeferCleanup(func(x bool) { vars.UseExternalDrainer = x }, vars.UseExternalDrainer)
675+
vars.UseExternalDrainer = true
676+
677+
discoverSriovReturn.Store(&[]sriovnetworkv1.InterfaceExt{
678+
{
679+
Name: "eno1",
680+
Driver: "ice",
681+
PciAddress: "0000:16:00.0",
682+
DeviceID: "1593",
683+
Vendor: "8086",
684+
EswitchMode: "legacy",
685+
LinkAdminState: "up",
686+
LinkSpeed: "10000 Mb/s",
687+
LinkType: "ETH",
688+
Mac: "aa:bb:cc:dd:ee:ff",
689+
Mtu: 1500,
690+
TotalVfs: 2,
691+
NumVfs: 0,
692+
},
693+
})
694+
695+
By("waiting for state to be in progress")
696+
eventuallySyncStatusEqual(nodeState, constants.SyncStatusInProgress)
697+
698+
err := k8sClient.Get(ctx, types.NamespacedName{Namespace: nodeState.Namespace, Name: nodeState.Name}, nodeState)
699+
Expect(err).ToNot(HaveOccurred())
700+
701+
By("waiting to require drain")
702+
EventuallyWithOffset(1, func(g Gomega) {
703+
g.Expect(k8sClient.Get(context.Background(), types.NamespacedName{Namespace: nodeState.Namespace, Name: nodeState.Name}, nodeState)).
704+
ToNot(HaveOccurred())
705+
706+
g.Expect(nodeState.Annotations[constants.NodeStateDrainAnnotation]).To(Equal(constants.DrainRequired))
707+
}, waitTime, retryTime).Should(Succeed())
708+
709+
// Validate status
710+
EventuallyWithOffset(1, func(g Gomega) {
711+
// get node by name
712+
node := &corev1.Node{}
713+
g.Expect(k8sClient.Get(context.Background(), types.NamespacedName{Name: nodeName}, node)).
714+
ToNot(HaveOccurred())
715+
g.Expect(nodeState.Annotations[constants.NodeStateExternalDrainerAnnotation]).To(Equal("true"))
716+
}, waitTime, retryTime).Should(Succeed())
717+
718+
patchAnnotation(nodeState, constants.NodeStateDrainAnnotationCurrent, constants.DrainComplete)
719+
// Validate status
720+
EventuallyWithOffset(1, func(g Gomega) {
721+
g.Expect(k8sClient.Get(context.Background(), types.NamespacedName{Namespace: nodeState.Namespace, Name: nodeState.Name}, nodeState)).
722+
ToNot(HaveOccurred())
723+
724+
g.Expect(nodeState.Status.SyncStatus).To(Equal(constants.SyncStatusInProgress))
725+
node := &corev1.Node{}
726+
g.Expect(k8sClient.Get(context.Background(), types.NamespacedName{Name: nodeName}, node)).
727+
ToNot(HaveOccurred())
728+
// verify that external drainer annotation is removed
729+
g.Expect(nodeState.Annotations).NotTo(ContainElement(constants.NodeStateExternalDrainerAnnotation))
730+
}, waitTime, retryTime).Should(Succeed())
731+
732+
})
672733
})
673734
})
674735

0 commit comments

Comments
 (0)