bug: keep NCP on Helm upgrade

Creation of NicClusterPolicy was removed from the Helm Chart. In order to avoid deletion of the CR on Helm upgrade, a Helm hook adds a 'keep' annotation to the NCP if needed, before running the actual upgrade. Signed-off-by: Fred Rolland <[email protected]>
rollandf · Nov 5, 2024 · 1b76032 · 1b76032
1 parent 01401aa
commit 1b76032
Show file tree

Hide file tree

Showing 5 changed files with 374 additions and 1 deletion.
diff --git a/Dockerfile b/Dockerfile
@@ -37,7 +37,9 @@ ARG LDFLAGS
 ARG GCFLAGS
 RUN --mount=type=cache,target=/go/pkg/mod \
     --mount=type=cache,target=/root/.cache/go-build \
-    CGO_ENABLED=0 GOOS=linux GOARCH=${ARCH} go build -ldflags="${LDFLAGS}" -gcflags="${GCFLAGS}" -o manager main.go
+    CGO_ENABLED=0 GOOS=linux GOARCH=${ARCH} go build -ldflags="${LDFLAGS}" -gcflags="${GCFLAGS}" -o manager main.go  && \
+    CGO_ENABLED=0 GOOS=linux GOARCH=${ARCH} go build -ldflags="${LDFLAGS}" -gcflags="${GCFLAGS}" -o keep-ncp cmd/keep-ncp/main.go
+
 
 # Build the apply-crds binary
 FROM golang:1.23@sha256:ad5c126b5cf501a8caef751a243bb717ec204ab1aa56dc41dc11be089fafcb4f AS apply-crds-builder
@@ -75,6 +77,7 @@ FROM --platform=linux/${ARCH} registry.access.redhat.com/ubi8-micro:8.10
 
 WORKDIR /
 COPY --from=manager-builder /workspace/manager .
+COPY --from=manager-builder /workspace/keep-ncp .
 COPY --from=apply-crds-builder /workspace/apply-crds .
 COPY --from=apply-crds-builder /workspace/crds /crds
 

diff --git a/cmd/keep-ncp/main.go b/cmd/keep-ncp/main.go
@@ -0,0 +1,97 @@
+/*
+2024 NVIDIA CORPORATION & AFFILIATES
+
+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 main adds Helm 'keep' annotation on NCP if needed
+package main
+
+import (
+	"context"
+	"log"
+
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime"
+	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	mellanoxv1alpha1 "github.com/Mellanox/network-operator/api/v1alpha1"
+	"github.com/Mellanox/network-operator/pkg/consts"
+)
+
+const (
+	helmKeepValue         = "keep"
+	helmResourcePolicyKey = "helm.sh/resource-policy"
+)
+
+func main() {
+	ctx := context.Background()
+	scheme := runtime.NewScheme()
+	utilruntime.Must(mellanoxv1alpha1.AddToScheme(scheme))
+
+	config, err := ctrl.GetConfig()
+	if err != nil {
+		log.Fatalf("Failed to get Kubernetes config: %v", err)
+	}
+
+	c, err := client.New(config, client.Options{Scheme: scheme})
+	if err != nil {
+		log.Fatalf("Error creating controller-runtime client: %v", err)
+	}
+
+	if err := annotateNCP(ctx, c); err != nil {
+		log.Fatalf("Failed to annotate NicClusterPolicy: %v", err)
+	}
+}
+
+// annotateNCP annotate NicClusterPolicy with "helm.sh/resource-policy=keep" if needed
+func annotateNCP(ctx context.Context, c client.Client) error {
+	ncp := &mellanoxv1alpha1.NicClusterPolicy{}
+	key := client.ObjectKey{
+		Name: consts.NicClusterPolicyResourceName,
+	}
+	err := c.Get(ctx, key, ncp)
+	if apierrors.IsNotFound(err) {
+		log.Println("NicClusterPolicy does not exists. No annotation needed")
+		return nil
+	}
+	if err != nil {
+		log.Println("Failed to get NicClusterPolicy")
+		return err
+	}
+	labels := ncp.GetLabels()
+	if labels["app.kubernetes.io/managed-by"] != "Helm" {
+		log.Println("NicClusterPolicy is not managed by Helm. No annotation needed")
+		return nil
+	}
+	if ncp.Annotations != nil {
+		val, ok := ncp.Annotations[helmResourcePolicyKey]
+		if ok && val == helmKeepValue {
+			log.Println("NicClusterPolicy already have keep annotation")
+			return nil
+		}
+	}
+	if ncp.Annotations == nil {
+		ncp.Annotations = make(map[string]string)
+	}
+	ncp.Annotations[helmResourcePolicyKey] = helmKeepValue
+
+	err = c.Update(ctx, ncp)
+	if err != nil {
+		log.Println("Error updating NicClusterPolicy with 'keep' annotation")
+		return err
+	}
+	return nil
+}
diff --git a/cmd/keep-ncp/main_test.go b/cmd/keep-ncp/main_test.go
@@ -0,0 +1,105 @@
+/*
+2024 NVIDIA CORPORATION & AFFILIATES
+
+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 main
+
+import (
+	"context"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+
+	mellanoxv1alpha1 "github.com/Mellanox/network-operator/api/v1alpha1"
+	"github.com/Mellanox/network-operator/pkg/consts"
+)
+
+var _ = Describe("NCP keep annotation", func() {
+	var (
+		ctx context.Context
+	)
+
+	BeforeEach(func() {
+		ctx = context.Background()
+	})
+
+	AfterEach(func() {
+		ncp := &mellanoxv1alpha1.NicClusterPolicy{
+			ObjectMeta: metav1.ObjectMeta{Namespace: "", Name: consts.NicClusterPolicyResourceName}}
+		_ = k8sClient.Delete(ctx, ncp)
+	})
+
+	Describe("annotate keep NCP", func() {
+		It("should succeed - NCP does not exists", func() {
+			Expect(annotateNCP(ctx, k8sClient)).To(Succeed())
+		})
+		It("should succeed - NCP managed by Helm", func() {
+			createNcp(true, false)
+			Expect(annotateNCP(ctx, k8sClient)).To(Succeed())
+			Eventually(func() bool {
+				found := &mellanoxv1alpha1.NicClusterPolicy{}
+				err := k8sClient.Get(context.TODO(),
+					types.NamespacedName{Name: consts.NicClusterPolicyResourceName}, found)
+				Expect(err).NotTo(HaveOccurred())
+				val, ok := found.Annotations[helmResourcePolicyKey]
+				return ok && val == helmKeepValue
+			}, timeout*3, interval).Should(BeTrue())
+		})
+		It("should succeed - NCP not managed by Helm", func() {
+			createNcp(false, false)
+			Expect(annotateNCP(ctx, k8sClient)).To(Succeed())
+			Eventually(func() bool {
+				found := &mellanoxv1alpha1.NicClusterPolicy{}
+				err := k8sClient.Get(context.TODO(),
+					types.NamespacedName{Name: consts.NicClusterPolicyResourceName}, found)
+				Expect(err).NotTo(HaveOccurred())
+				_, ok := found.Annotations[helmResourcePolicyKey]
+				return ok
+			}, timeout*3, interval).Should(BeFalse())
+		})
+		It("should succeed - NCP managed by Helm, keep already set", func() {
+			createNcp(true, true)
+			Expect(annotateNCP(ctx, k8sClient)).To(Succeed())
+			Consistently(func() bool {
+				found := &mellanoxv1alpha1.NicClusterPolicy{}
+				err := k8sClient.Get(context.TODO(),
+					types.NamespacedName{Name: consts.NicClusterPolicyResourceName}, found)
+				Expect(err).NotTo(HaveOccurred())
+				val, ok := found.Annotations[helmResourcePolicyKey]
+				return ok && val == helmKeepValue
+			}, timeout, interval).Should(BeTrue())
+		})
+	})
+})
+
+func createNcp(helmManaged, keepAnnotation bool) {
+	cr := mellanoxv1alpha1.NicClusterPolicy{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      consts.NicClusterPolicyResourceName,
+			Namespace: "",
+		},
+	}
+	if helmManaged {
+		cr.Labels = map[string]string{"app.kubernetes.io/managed-by": "Helm"}
+	}
+	if keepAnnotation {
+		cr.Annotations = map[string]string{helmResourcePolicyKey: helmKeepValue}
+	}
+	err := k8sClient.Create(context.TODO(), &cr)
+	Expect(err).NotTo(HaveOccurred())
+}
diff --git a/cmd/keep-ncp/suite_test.go b/cmd/keep-ncp/suite_test.go
@@ -0,0 +1,84 @@
+/*
+2024 NVIDIA CORPORATION & AFFILIATES
+
+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 main
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"k8s.io/client-go/kubernetes/scheme"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/envtest"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
+	"sigs.k8s.io/controller-runtime/pkg/log/zap"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+
+	mellanoxcomv1alpha1 "github.com/Mellanox/network-operator/api/v1alpha1"
+)
+
+var (
+	testEnv   *envtest.Environment
+	k8sClient client.Client
+)
+
+const (
+	timeout  = time.Second * 10
+	interval = time.Millisecond * 250
+)
+
+func TestApplyCrds(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Annotate Keep NCP Suite")
+}
+
+var _ = BeforeSuite(func() {
+	logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true)))
+
+	// Go to project root directory
+	er := os.Chdir("../..")
+	Expect(er).NotTo(HaveOccurred())
+
+	By("bootstrapping test environment")
+	testEnv = &envtest.Environment{
+		CRDDirectoryPaths: []string{filepath.Join("config", "crd", "bases")},
+	}
+
+	cfg, err := testEnv.Start()
+	Expect(err).NotTo(HaveOccurred())
+	Expect(cfg).NotTo(BeNil())
+
+	err = mellanoxcomv1alpha1.AddToScheme(scheme.Scheme)
+	Expect(err).NotTo(HaveOccurred())
+
+	k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme})
+	Expect(err).NotTo(HaveOccurred())
+	Expect(k8sClient).NotTo(BeNil())
+
+	go func() {
+		defer GinkgoRecover()
+	}()
+})
+
+var _ = AfterSuite(func() {
+	By("tearing down the test environment")
+	err := testEnv.Stop()
+	Expect(err).NotTo(HaveOccurred())
+})
diff --git a/deployment/network-operator/templates/keep-ncp-hook.yaml b/deployment/network-operator/templates/keep-ncp-hook.yaml
@@ -0,0 +1,84 @@
+---
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: {{ include "network-operator.fullname" . }}-hooks-keep-ncp-sa
+  annotations:
+    helm.sh/hook: pre-install,pre-upgrade
+    helm.sh/hook-delete-policy: hook-succeeded,before-hook-creation
+    helm.sh/hook-weight: "0"
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+  name: {{ include "network-operator.fullname" . }}-hooks-keep-ncp-role
+  annotations:
+    helm.sh/hook: pre-install,pre-upgrade
+    helm.sh/hook-delete-policy: hook-succeeded,before-hook-creation
+    helm.sh/hook-weight: "0"
+rules:
+  - apiGroups:
+      - mellanox.com
+    resources:
+      - nicclusterpolicies
+    verbs:
+      - "*"
+---
+kind: ClusterRoleBinding
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: {{ include "network-operator.fullname" . }}-hooks-scale-binding
+  annotations:
+    helm.sh/hook: pre-install,pre-upgrade
+    helm.sh/hook-delete-policy: hook-succeeded,before-hook-creation
+    helm.sh/hook-weight: "0"
+subjects:
+  - kind: ServiceAccount
+    name: {{ include "network-operator.fullname" . }}-hooks-keep-ncp-sa
+    namespace: {{ .Release.Namespace }}
+roleRef:
+  kind: ClusterRole
+  name: {{ include "network-operator.fullname" . }}-hooks-keep-ncp-role
+  apiGroup: rbac.authorization.k8s.io
+---
+apiVersion: batch/v1
+kind: Job
+metadata:
+  name: network-operator-keep-ncp
+  namespace: {{ .Release.Namespace }}
+  annotations:
+    "helm.sh/hook": pre-install,pre-upgrade
+    "helm.sh/hook-weight": "1"
+    "helm.sh/hook-delete-policy": hook-succeeded,before-hook-creation
+  labels:
+    {{- include "network-operator.labels" . | nindent 4 }}
+    app.kubernetes.io/component: "network-operator"
+spec:
+  template:
+    metadata:
+      name: network-operator-keep-ncp
+      labels:
+        {{- include "network-operator.labels" . | nindent 8 }}
+        app.kubernetes.io/component: "network-operator"
+    spec:
+      {{- with .Values.operator.nodeSelector }}
+      nodeSelector:
+      {{- toYaml . | nindent 8 }}
+      {{- end }}
+      {{- with .Values.operator.affinity}}
+      affinity:
+      {{- toYaml . | nindent 8 }}
+      {{- end }}
+      {{- with .Values.operator.tolerations }}
+      tolerations:
+      {{- toYaml . | nindent 8 }}
+      {{- end }}
+      serviceAccountName: {{ include "network-operator.fullname" . }}-hooks-keep-ncp-sa
+      imagePullSecrets: {{ include "network-operator.operator.imagePullSecrets" . }}
+      containers:
+        - name: keep-ncp
+          image: "{{ .Values.operator.repository }}/{{ .Values.operator.image }}:{{ .Values.operator.tag | default .Chart.AppVersion }}"
+          imagePullPolicy: IfNotPresent
+          command:
+            - /keep-ncp
+      restartPolicy: OnFailure