Revert Metal3DataTemplate templateReference logic, tests and docs

Signed-off-by: peppi-lotta <[email protected]>

Author: peppi-lotta

api/v1beta1/metal3data_webhook.go

@@ -43,8 +43,7 @@ func (c *Metal3Data) Default() {
 // ValidateCreate implements webhook.Validator so a webhook will be registered for the type.
 func (c *Metal3Data) ValidateCreate() (admission.Warnings, error) {
 	allErrs := field.ErrorList{}
-	if (c.Spec.TemplateReference != "" && c.Name != c.Spec.TemplateReference+"-"+strconv.Itoa(c.Spec.Index)) ||
-		(c.Spec.TemplateReference == "" && c.Name != c.Spec.Template.Name+"-"+strconv.Itoa(c.Spec.Index)) {
+	if c.Name != c.Spec.Template.Name+"-"+strconv.Itoa(c.Spec.Index) {
 		allErrs = append(allErrs,
 			field.Invalid(
 				field.NewPath("name"),

api/v1beta1/metal3datatemplate_types.go

@@ -518,12 +518,6 @@ type Metal3DataTemplateSpec struct {
 	// +kubebuilder:validation:MinLength=1
 	ClusterName string `json:"clusterName"`
 
-	// TemplateReference refers to the Template the Metal3MachineTemplate refers to.
-	// It can be matched against the key or it may also point to the name of the template
-	// Metal3Data refers to
-	// +optional
-	TemplateReference string `json:"templateReference,omitempty"`
-
 	// MetaData contains the information needed to generate the metadata secret
 	// +optional
 	MetaData *MetaData `json:"metaData,omitempty"`

baremetal/metal3datatemplate_manager.go

@@ -127,8 +127,7 @@ func (m *DataTemplateManager) getIndexes(ctx context.Context) (map[int]string, e
 		if dataObject.Spec.Template.Name == "" {
 			continue
 		}
-
-		if !m.dataObjectBelongsToTemplate(dataObject) {
+		if dataObject.Spec.Template.Name != m.DataTemplate.Name {
 			continue
 		}
 
@@ -140,22 +139,6 @@ func (m *DataTemplateManager) getIndexes(ctx context.Context) (map[int]string, e
 	return indexes, nil
 }
 
-func (m *DataTemplateManager) dataObjectBelongsToTemplate(dataObject infrav1.Metal3Data) bool {
-	if dataObject.Spec.Template.Name == m.DataTemplate.Name {
-		return true
-	}
-
-	// Match TemplateReference
-	if dataObject.Spec.TemplateReference == "" && m.DataTemplate.Spec.TemplateReference == dataObject.Spec.Template.Name {
-		return true
-	}
-
-	if dataObject.Spec.TemplateReference != "" && m.DataTemplate.Spec.TemplateReference == dataObject.Spec.TemplateReference {
-		return true
-	}
-	return false
-}
-
 func (m *DataTemplateManager) updateStatusTimestamp() {
 	now := metav1.Now()
 	m.DataTemplate.Status.LastUpdated = &now
@@ -239,21 +222,13 @@ func (m *DataTemplateManager) updateData(ctx context.Context,
 func (m *DataTemplateManager) createData(ctx context.Context,
 	dataClaim *infrav1.Metal3DataClaim, indexes map[int]string,
 ) (map[int]string, error) {
-	var dataName string
-
 	if !controllerutil.ContainsFinalizer(dataClaim, infrav1.DataClaimFinalizer) {
 		controllerutil.AddFinalizer(dataClaim, infrav1.DataClaimFinalizer)
 	}
 
 	if dataClaimIndex, ok := m.DataTemplate.Status.Indexes[dataClaim.Name]; ok {
-		if m.DataTemplate.Spec.TemplateReference != "" {
-			dataName = m.DataTemplate.Spec.TemplateReference + "-" + strconv.Itoa(dataClaimIndex)
-		} else {
-			dataName = m.DataTemplate.Name + "-" + strconv.Itoa(dataClaimIndex)
-		}
-
 		dataClaim.Status.RenderedData = &corev1.ObjectReference{
-			Name:      dataName,
+			Name:      m.DataTemplate.Name + "-" + strconv.Itoa(dataClaimIndex),
 			Namespace: m.DataTemplate.Namespace,
 		}
 		return indexes, nil
@@ -292,11 +267,8 @@ func (m *DataTemplateManager) createData(ctx context.Context,
 	}
 
 	// Set the index and Metal3Data names
-	if m.DataTemplate.Spec.TemplateReference != "" {
-		dataName = m.DataTemplate.Spec.TemplateReference + "-" + strconv.Itoa(claimIndex)
-	} else {
-		dataName = m.DataTemplate.Name + "-" + strconv.Itoa(claimIndex)
-	}
+	dataName := m.DataTemplate.Name + "-" + strconv.Itoa(claimIndex)
+
 	m.Log.Info("Index", "Claim", dataClaim.Name, "index", claimIndex)
 
 	// Create the Metal3Data object, with an Owner ref to the Metal3Machine
@@ -334,8 +306,7 @@ func (m *DataTemplateManager) createData(ctx context.Context,
 			},
 		},
 		Spec: infrav1.Metal3DataSpec{
-			Index:             claimIndex,
-			TemplateReference: m.DataTemplate.Spec.TemplateReference,
+			Index: claimIndex,
 			Template: corev1.ObjectReference{
 				Name:      m.DataTemplate.Name,
 				Namespace: m.DataTemplate.Namespace,
@@ -379,16 +350,8 @@ func (m *DataTemplateManager) deleteData(ctx context.Context,
 	if ok {
 		// Try to get the Metal3Data. if it succeeds, delete it
 		tmpM3Data := &infrav1.Metal3Data{}
-
-		var dataName string
-		if m.DataTemplate.Spec.TemplateReference != "" {
-			dataName = m.DataTemplate.Spec.TemplateReference + "-" + strconv.Itoa(dataClaimIndex)
-		} else {
-			dataName = m.DataTemplate.Name + "-" + strconv.Itoa(dataClaimIndex)
-		}
-
 		key := client.ObjectKey{
-			Name:      dataName,
+			Name:      m.DataTemplate.Name + "-" + strconv.Itoa(dataClaimIndex),
 			Namespace: m.DataTemplate.Namespace,
 		}
 		err := m.client.Get(ctx, key, tmpM3Data)

baremetal/metal3datatemplate_manager_test.go

@@ -18,7 +18,6 @@ package baremetal
 
 import (
 	"context"
-	"strconv"
 
 	"github.com/go-logr/logr"
 	infrav1 "github.com/metal3-io/cluster-api-provider-metal3/api/v1beta1"
@@ -427,173 +426,6 @@ var _ = Describe("Metal3DataTemplate manager", func() {
 		}),
 	)
 
-	type testCaseTemplateReference struct {
-		template1                  *infrav1.Metal3DataTemplate
-		template2                  *infrav1.Metal3DataTemplate
-		dataObject                 *infrav1.Metal3Data
-		dataClaim                  *infrav1.Metal3DataClaim
-		indexes                    map[int]string
-		expectError                bool
-		expectTemplateReference    bool
-		expectDataObjectAssociated bool
-	}
-
-	DescribeTable("Test Template Reference",
-		func(tc testCaseTemplateReference) {
-			objects := []client.Object{}
-			objects = append(objects, tc.dataClaim)
-			if tc.dataObject != nil {
-				objects = append(objects, tc.dataObject)
-			}
-			fakeClient := fake.NewClientBuilder().WithScheme(setupSchemeMm()).WithObjects(objects...).Build()
-			templateMgr, err := NewDataTemplateManager(fakeClient, tc.template2,
-				logr.Discard(),
-			)
-			Expect(err).NotTo(HaveOccurred())
-
-			_, err = templateMgr.createData(context.TODO(), tc.dataClaim, tc.indexes)
-			if tc.expectError {
-				Expect(err).To(HaveOccurred())
-			} else {
-				Expect(err).NotTo(HaveOccurred())
-			}
-
-			dataObjects := infrav1.Metal3DataList{}
-			opts := &client.ListOptions{}
-			err = fakeClient.List(context.TODO(), &dataObjects, opts)
-			Expect(err).NotTo(HaveOccurred())
-			if tc.dataObject != nil {
-				Expect(dataObjects.Items).To(HaveLen(2))
-			} else {
-				Expect(dataObjects.Items).To(HaveLen(1))
-			}
-
-			if tc.expectTemplateReference {
-				Expect(dataObjects.Items[0].Spec.TemplateReference).To(Equal(tc.template1.Name))
-			} else {
-				Expect(dataObjects.Items[0].Spec.TemplateReference).ToNot(Equal(tc.template1.Name))
-			}
-
-			if tc.dataObject != nil {
-				if tc.expectDataObjectAssociated {
-					result := templateMgr.dataObjectBelongsToTemplate(*tc.dataObject)
-					Expect(result).To(BeTrue())
-					dataClaimIndex := tc.template1.Status.Indexes[tc.dataClaim.ObjectMeta.Name]
-					Expect(tc.dataObject.ObjectMeta.Name).To(Equal(
-						tc.template1.ObjectMeta.Name + "-" + strconv.Itoa(dataClaimIndex)))
-				} else {
-					result := templateMgr.dataObjectBelongsToTemplate(*tc.dataObject)
-					Expect(result).To(BeFalse())
-					dataClaimIndex := tc.template1.Status.Indexes[tc.dataClaim.ObjectMeta.Name]
-					Expect(tc.dataObject.ObjectMeta.Name).ToNot(Equal(tc.template1.ObjectMeta.Name + "-" + strconv.Itoa(dataClaimIndex)))
-				}
-			}
-		},
-		Entry("TemplateReferenceExist", testCaseTemplateReference{
-			template1: &infrav1.Metal3DataTemplate{
-				ObjectMeta: templateMeta,
-				Spec:       infrav1.Metal3DataTemplateSpec{},
-			},
-			indexes: map[int]string{},
-			template2: &infrav1.Metal3DataTemplate{
-				ObjectMeta: testObjectMeta("abc1", namespaceName, ""),
-				Spec: infrav1.Metal3DataTemplateSpec{
-					TemplateReference: "abc",
-				},
-				Status: infrav1.Metal3DataTemplateStatus{
-					Indexes: map[string]int{},
-				},
-			},
-			dataClaim: &infrav1.Metal3DataClaim{
-				ObjectMeta: testObjectMetaWithOR(metal3DataClaimName, metal3machineName),
-			},
-			expectTemplateReference: true,
-		}),
-		Entry("TemplateReferenceDoNotExist", testCaseTemplateReference{
-			template1: &infrav1.Metal3DataTemplate{
-				ObjectMeta: templateMeta,
-				Spec:       infrav1.Metal3DataTemplateSpec{},
-			},
-			indexes: map[int]string{},
-			template2: &infrav1.Metal3DataTemplate{
-				ObjectMeta: testObjectMeta("abc1", namespaceName, ""),
-				Spec:       infrav1.Metal3DataTemplateSpec{},
-				Status: infrav1.Metal3DataTemplateStatus{
-					Indexes: map[string]int{},
-				},
-			},
-			dataClaim: &infrav1.Metal3DataClaim{
-				ObjectMeta: testObjectMetaWithOR(metal3DataClaimName, metal3machineName),
-			},
-			expectTemplateReference: false,
-		}),
-		Entry("TemplateReferenceRefersToOldTemplate", testCaseTemplateReference{
-			template1: &infrav1.Metal3DataTemplate{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "template1",
-					Namespace: namespaceName,
-				},
-				Spec: infrav1.Metal3DataTemplateSpec{},
-			},
-			indexes: map[int]string{},
-			template2: &infrav1.Metal3DataTemplate{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "template2",
-					Namespace: namespaceName,
-				},
-				Spec: infrav1.Metal3DataTemplateSpec{
-					TemplateReference: "template1",
-				},
-				Status: infrav1.Metal3DataTemplateStatus{
-					Indexes: map[string]int{},
-				},
-			},
-			dataClaim: &infrav1.Metal3DataClaim{
-				ObjectMeta: testObjectMetaWithOR(metal3DataClaimName, metal3machineName),
-			},
-			expectTemplateReference:    true,
-			expectDataObjectAssociated: true,
-		}),
-		Entry("TemplateReferenceRefersToZombieTemplate", testCaseTemplateReference{
-			template1: &infrav1.Metal3DataTemplate{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "template1",
-					Namespace: namespaceName,
-				},
-				Spec: infrav1.Metal3DataTemplateSpec{},
-			},
-			indexes: map[int]string{},
-			template2: &infrav1.Metal3DataTemplate{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "template2",
-					Namespace: namespaceName,
-				},
-				Spec: infrav1.Metal3DataTemplateSpec{
-					TemplateReference: "template1",
-				},
-				Status: infrav1.Metal3DataTemplateStatus{
-					Indexes: map[string]int{},
-				},
-			},
-			dataClaim: &infrav1.Metal3DataClaim{
-				ObjectMeta: testObjectMetaWithOR(metal3DataClaimName, metal3machineName),
-			},
-			dataObject: &infrav1.Metal3Data{
-				ObjectMeta: testObjectMeta(metal3DataName, namespaceName, ""),
-				Spec: infrav1.Metal3DataSpec{
-					Index: 0,
-					Template: corev1.ObjectReference{
-						Name: "template12",
-					},
-					Claim: corev1.ObjectReference{
-						Name: "abc",
-					},
-				},
-			},
-			expectDataObjectAssociated: false,
-		}),
-	)
-
 	type testCaseCreateAddresses struct {
 		template        *infrav1.Metal3DataTemplate
 		dataClaim       *infrav1.Metal3DataClaim

config/crd/bases/infrastructure.cluster.x-k8s.io_metal3datas.yaml

@@ -170,12 +170,6 @@ spec:
                     type: string
                 type: object
                 x-kubernetes-map-type: atomic
-              templateReference:
-                description: |-
-                  TemplateReference refers to the Template the Metal3MachineTemplate refers to.
-                  It can be matched against the key or it may also point to the name of the template
-                  Metal3Data refers to
-                type: string
             required:
             - claim
             - template

config/crd/bases/infrastructure.cluster.x-k8s.io_metal3datatemplates.yaml

@@ -986,12 +986,6 @@ spec:
                         type: string
                     type: object
                 type: object
-              templateReference:
-                description: |-
-                  TemplateReference refers to the Template the Metal3MachineTemplate refers to.
-                  It can be matched against the key or it may also point to the name of the template
-                  Metal3Data refers to
-                type: string
             required:
             - clusterName
             type: object

docs/api.md

@@ -591,7 +591,6 @@ metadata:
     kind: Metal3Cluster
     name: cluster-1
 spec:
-  templateReference: old-template
   metaData:
     strings:
     - key: abc
@@ -893,45 +892,6 @@ The object for the **services** section can be:
 - **dns**: a list of dns service with the ip address of a dns server
 - **dnsFromIPPool**: the IPPool from which to fetch the dns servers list
 
-#### Updating metaData and networkData
-
-The data template parts containing the metadata and networkData must be
-immutable since the BareMetalHost references the secrets and they are used at
-provisioning time, the secrets cannot be updated to be able to reprovision the
-node in the exact same state. This means that updates have to be done by
-creating a new template and referencing it in the new/updated
-Metal3MachineTemplate.
-
-The process to allow updating the metaData and networkData is then to create a
-new Metal3DataTemplate and reference the new one in the Metal3MachineTemplate.
-This requires the Metal3Data to be linked to both Metal3DataTemplates. This is
-achieved using the `templateReference` field of the Metal3DataTemplate. When a
-Metal3Data corresponding to the Metal3DataTemplate is created, the reconciler
-will set the `templateReference` to the value of the Metal3DataTemplate, if set.
-
-The Metal3Data objects are linked to a Metal3DataTemplate by three ways:
-
-- Directly reference the template in the `template` field of the Metal3Data
-  `spec`
-- They have the same `templateReference` key as the template
-- The template's `templateReference` matches the `name` of the `template` field
-  of the `spec` of the Metal3Data.
-
-The third way ensures backward compatibility with previous version of
-implementation when there was no `templateReference` in Metal3Data objects.
-Since the `templateReference` field is an addition to the existing API, the
-default behaviour of the controller will be to list the Metal3Data objects by
-matching their template field if the `templateReference` is left empty on the
-template object. However, if the `templateReference` is set on the
-Metal3DataTemplate object, but not on the Metal3Data object, then the controller
-will match the `templateReference` field with the `name` of the `template` field
-of the Metal3Data object. This is performed by setting the `templateReference`
-value on a new Metal3DataTemplate object to the name of an old
-Metal3DataTemplate object. This allows to transition from the old template
-object, which is without `templateReference` set on the Metal3Data objects
-created from the old template object to the new one which uses the
-`templateReference`.
-
 ## The Metal3DataClaim object
 
 A new object would be created, a Metal3DataClaim type.
@@ -980,7 +940,6 @@ metadata:
     kind: Metal3DataTemplate
     name: nodepool-1
 spec:
-  templateReference: old-template
   index: 0
   claim:
     name: machine-1