Allow pinned NnfStorageProfiles to be updated

The NnfStorageProfiles for the NnfSystemStorage resources are created directly from manifests in the gitops repo where they are already marked as pinned. Allow these to be updated during a software upgrade. Signed-off-by: Dean Roehrich <[email protected]>
NearNodeFlash · Dec 11, 2024 · 122bdae · 122bdae
1 parent e03b38f
commit 122bdae
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 17 deletions.
diff --git a/api/v1alpha4/nnfstorageprofile_types.go b/api/v1alpha4/nnfstorageprofile_types.go
@@ -267,7 +267,7 @@ type NnfStorageProfileData struct {
 	// +kubebuilder:default:=false
 	Default bool `json:"default,omitempty"`
 
-	// Pinned is true if this instance is an immutable copy
+	// Pinned is true if this instance is describing an active storage resource
 	// +kubebuilder:default:=false
 	Pinned bool `json:"pinned,omitempty"`
 

diff --git a/api/v1alpha4/nnfstorageprofile_webhook.go b/api/v1alpha4/nnfstorageprofile_webhook.go
@@ -22,7 +22,6 @@ package v1alpha4
 import (
 	"fmt"
 	"os"
-	"reflect"
 
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -76,17 +75,11 @@ func (r *NnfStorageProfile) ValidateUpdate(old runtime.Object) (admission.Warnin
 		nnfstorageprofilelog.Error(err, "invalid")
 		return nil, err
 	}
-	if obj.Data.Pinned {
-		// Allow metadata to be updated, for things like finalizers,
-		// ownerReferences, and labels, but do not allow Data to be
-		// updated.
-		if !reflect.DeepEqual(r.Data, obj.Data) {
-			msg := "update on pinned resource not allowed"
-			err := fmt.Errorf(msg)
-			nnfstorageprofilelog.Error(err, "invalid")
-			return nil, err
-		}
-	}
+
+	// WARNING: NnfStorageProfile allows the obj.Data section to be modified.
+	// This is the place in the webhook where our other profile types, such as
+	// NnfContainerProfile or NnfDataMovementProfile, would verify that their
+	// obj.Data has not been modified.
 
 	if err := r.validateContent(); err != nil {
 		nnfstorageprofilelog.Error(err, "invalid NnfStorageProfile resource")

diff --git a/api/v1alpha4/nnfstorageprofile_webhook_test.go b/api/v1alpha4/nnfstorageprofile_webhook_test.go
@@ -172,10 +172,14 @@ var _ = Describe("NnfStorageProfile Webhook", func() {
 		nnfProfile = nil
 	})
 
-	It("Should not allow modification of Data in a pinned resource", func() {
+	It("Should allow modification of Data in a pinned resource", func() {
+		// WARNING: Our other profile types, such as NnfContainerProfile or
+		// NnfDataMovementProfile, do not allow this.
+
 		nnfProfile.ObjectMeta.Name = pinnedResourceName
 		nnfProfile.ObjectMeta.Namespace = otherNamespaceName
 		nnfProfile.Data.Pinned = true
+		nnfProfile.Data.RawStorage.CmdLines.LvRemove = "lvremove $VG_NAME"
 
 		Expect(k8sClient.Create(context.TODO(), nnfProfile)).To(Succeed())
 		Eventually(func() error {
@@ -184,8 +188,8 @@ var _ = Describe("NnfStorageProfile Webhook", func() {
 
 		Expect(k8sClient.Get(context.TODO(), client.ObjectKeyFromObject(nnfProfile), newProfile)).To(Succeed())
 		Expect(newProfile.Data.Pinned).To(BeTrue())
-		newProfile.Data.Pinned = false
-		Expect(k8sClient.Update(context.TODO(), newProfile)).ToNot(Succeed())
+		newProfile.Data.RawStorage.CmdLines.LvRemove = "lvremove $VG_NAME/$LV_NAME"
+		Expect(k8sClient.Update(context.TODO(), newProfile)).To(Succeed())
 	})
 
 	It("Should allow modification of Meta in a pinned resource", func() {

diff --git a/config/crd/bases/nnf.cray.hpe.com_nnfstorageprofiles.yaml b/config/crd/bases/nnf.cray.hpe.com_nnfstorageprofiles.yaml
@@ -1784,7 +1784,8 @@ spec:
                 type: object
               pinned:
                 default: false
-                description: Pinned is true if this instance is an immutable copy
+                description: Pinned is true if this instance is describing an active
+                  storage resource
                 type: boolean
               rawStorage:
                 description: RawStorage defines the Raw-specific configuration