Skip to content

Commit 1503da1

Browse files
peppi-lottapeppi-lotta
committed
Revert Metal3DataTemplate templateReference logic, tests and docs
Signed-off-by: peppi-lotta <[email protected]>
1 parent 7aaf7b8 commit 1503da1

File tree

5 files changed

+8
-274
lines changed

5 files changed

+8
-274
lines changed

‎api/v1beta1/metal3data_webhook.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,7 @@ func (c *Metal3Data) Default() {
4343
// ValidateCreate implements webhook.Validator so a webhook will be registered for the type.
4444
func (c *Metal3Data) ValidateCreate() (admission.Warnings, error) {
4545
allErrs := field.ErrorList{}
46-
if (c.Spec.TemplateReference != "" && c.Name != c.Spec.TemplateReference+"-"+strconv.Itoa(c.Spec.Index)) ||
47-
(c.Spec.TemplateReference == "" && c.Name != c.Spec.Template.Name+"-"+strconv.Itoa(c.Spec.Index)) {
46+
if c.Name != c.Spec.Template.Name+"-"+strconv.Itoa(c.Spec.Index) {
4847
allErrs = append(allErrs,
4948
field.Invalid(
5049
field.NewPath("name"),

‎baremetal/metal3datatemplate_manager.go

+6-43
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,7 @@ func (m *DataTemplateManager) getIndexes(ctx context.Context) (map[int]string, e
127127
if dataObject.Spec.Template.Name == "" {
128128
continue
129129
}
130-
131-
if !m.dataObjectBelongsToTemplate(dataObject) {
130+
if dataObject.Spec.Template.Name != m.DataTemplate.Name {
132131
continue
133132
}
134133

@@ -140,22 +139,6 @@ func (m *DataTemplateManager) getIndexes(ctx context.Context) (map[int]string, e
140139
return indexes, nil
141140
}
142141

143-
func (m *DataTemplateManager) dataObjectBelongsToTemplate(dataObject infrav1.Metal3Data) bool {
144-
if dataObject.Spec.Template.Name == m.DataTemplate.Name {
145-
return true
146-
}
147-
148-
// Match TemplateReference
149-
if dataObject.Spec.TemplateReference == "" && m.DataTemplate.Spec.TemplateReference == dataObject.Spec.Template.Name {
150-
return true
151-
}
152-
153-
if dataObject.Spec.TemplateReference != "" && m.DataTemplate.Spec.TemplateReference == dataObject.Spec.TemplateReference {
154-
return true
155-
}
156-
return false
157-
}
158-
159142
func (m *DataTemplateManager) updateStatusTimestamp() {
160143
now := metav1.Now()
161144
m.DataTemplate.Status.LastUpdated = &now
@@ -239,21 +222,13 @@ func (m *DataTemplateManager) updateData(ctx context.Context,
239222
func (m *DataTemplateManager) createData(ctx context.Context,
240223
dataClaim *infrav1.Metal3DataClaim, indexes map[int]string,
241224
) (map[int]string, error) {
242-
var dataName string
243-
244225
if !controllerutil.ContainsFinalizer(dataClaim, infrav1.DataClaimFinalizer) {
245226
controllerutil.AddFinalizer(dataClaim, infrav1.DataClaimFinalizer)
246227
}
247228

248229
if dataClaimIndex, ok := m.DataTemplate.Status.Indexes[dataClaim.Name]; ok {
249-
if m.DataTemplate.Spec.TemplateReference != "" {
250-
dataName = m.DataTemplate.Spec.TemplateReference + "-" + strconv.Itoa(dataClaimIndex)
251-
} else {
252-
dataName = m.DataTemplate.Name + "-" + strconv.Itoa(dataClaimIndex)
253-
}
254-
255230
dataClaim.Status.RenderedData = &corev1.ObjectReference{
256-
Name: dataName,
231+
Name: m.DataTemplate.Name + "-" + strconv.Itoa(dataClaimIndex),
257232
Namespace: m.DataTemplate.Namespace,
258233
}
259234
return indexes, nil
@@ -292,11 +267,8 @@ func (m *DataTemplateManager) createData(ctx context.Context,
292267
}
293268

294269
// Set the index and Metal3Data names
295-
if m.DataTemplate.Spec.TemplateReference != "" {
296-
dataName = m.DataTemplate.Spec.TemplateReference + "-" + strconv.Itoa(claimIndex)
297-
} else {
298-
dataName = m.DataTemplate.Name + "-" + strconv.Itoa(claimIndex)
299-
}
270+
dataName := m.DataTemplate.Name + "-" + strconv.Itoa(claimIndex)
271+
300272
m.Log.Info("Index", "Claim", dataClaim.Name, "index", claimIndex)
301273

302274
// Create the Metal3Data object, with an Owner ref to the Metal3Machine
@@ -334,8 +306,7 @@ func (m *DataTemplateManager) createData(ctx context.Context,
334306
},
335307
},
336308
Spec: infrav1.Metal3DataSpec{
337-
Index: claimIndex,
338-
TemplateReference: m.DataTemplate.Spec.TemplateReference,
309+
Index: claimIndex,
339310
Template: corev1.ObjectReference{
340311
Name: m.DataTemplate.Name,
341312
Namespace: m.DataTemplate.Namespace,
@@ -379,16 +350,8 @@ func (m *DataTemplateManager) deleteData(ctx context.Context,
379350
if ok {
380351
// Try to get the Metal3Data. if it succeeds, delete it
381352
tmpM3Data := &infrav1.Metal3Data{}
382-
383-
var dataName string
384-
if m.DataTemplate.Spec.TemplateReference != "" {
385-
dataName = m.DataTemplate.Spec.TemplateReference + "-" + strconv.Itoa(dataClaimIndex)
386-
} else {
387-
dataName = m.DataTemplate.Name + "-" + strconv.Itoa(dataClaimIndex)
388-
}
389-
390353
key := client.ObjectKey{
391-
Name: dataName,
354+
Name: m.DataTemplate.Name + "-" + strconv.Itoa(dataClaimIndex),
392355
Namespace: m.DataTemplate.Namespace,
393356
}
394357
err := m.client.Get(ctx, key, tmpM3Data)

‎baremetal/metal3datatemplate_manager_test.go

-168
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ package baremetal
1818

1919
import (
2020
"context"
21-
"strconv"
2221

2322
"github.com/go-logr/logr"
2423
infrav1 "github.com/metal3-io/cluster-api-provider-metal3/api/v1beta1"
@@ -427,173 +426,6 @@ var _ = Describe("Metal3DataTemplate manager", func() {
427426
}),
428427
)
429428

430-
type testCaseTemplateReference struct {
431-
template1 *infrav1.Metal3DataTemplate
432-
template2 *infrav1.Metal3DataTemplate
433-
dataObject *infrav1.Metal3Data
434-
dataClaim *infrav1.Metal3DataClaim
435-
indexes map[int]string
436-
expectError bool
437-
expectTemplateReference bool
438-
expectDataObjectAssociated bool
439-
}
440-
441-
DescribeTable("Test Template Reference",
442-
func(tc testCaseTemplateReference) {
443-
objects := []client.Object{}
444-
objects = append(objects, tc.dataClaim)
445-
if tc.dataObject != nil {
446-
objects = append(objects, tc.dataObject)
447-
}
448-
fakeClient := fake.NewClientBuilder().WithScheme(setupSchemeMm()).WithObjects(objects...).Build()
449-
templateMgr, err := NewDataTemplateManager(fakeClient, tc.template2,
450-
logr.Discard(),
451-
)
452-
Expect(err).NotTo(HaveOccurred())
453-
454-
_, err = templateMgr.createData(context.TODO(), tc.dataClaim, tc.indexes)
455-
if tc.expectError {
456-
Expect(err).To(HaveOccurred())
457-
} else {
458-
Expect(err).NotTo(HaveOccurred())
459-
}
460-
461-
dataObjects := infrav1.Metal3DataList{}
462-
opts := &client.ListOptions{}
463-
err = fakeClient.List(context.TODO(), &dataObjects, opts)
464-
Expect(err).NotTo(HaveOccurred())
465-
if tc.dataObject != nil {
466-
Expect(dataObjects.Items).To(HaveLen(2))
467-
} else {
468-
Expect(dataObjects.Items).To(HaveLen(1))
469-
}
470-
471-
if tc.expectTemplateReference {
472-
Expect(dataObjects.Items[0].Spec.TemplateReference).To(Equal(tc.template1.Name))
473-
} else {
474-
Expect(dataObjects.Items[0].Spec.TemplateReference).ToNot(Equal(tc.template1.Name))
475-
}
476-
477-
if tc.dataObject != nil {
478-
if tc.expectDataObjectAssociated {
479-
result := templateMgr.dataObjectBelongsToTemplate(*tc.dataObject)
480-
Expect(result).To(BeTrue())
481-
dataClaimIndex := tc.template1.Status.Indexes[tc.dataClaim.ObjectMeta.Name]
482-
Expect(tc.dataObject.ObjectMeta.Name).To(Equal(
483-
tc.template1.ObjectMeta.Name + "-" + strconv.Itoa(dataClaimIndex)))
484-
} else {
485-
result := templateMgr.dataObjectBelongsToTemplate(*tc.dataObject)
486-
Expect(result).To(BeFalse())
487-
dataClaimIndex := tc.template1.Status.Indexes[tc.dataClaim.ObjectMeta.Name]
488-
Expect(tc.dataObject.ObjectMeta.Name).ToNot(Equal(tc.template1.ObjectMeta.Name + "-" + strconv.Itoa(dataClaimIndex)))
489-
}
490-
}
491-
},
492-
Entry("TemplateReferenceExist", testCaseTemplateReference{
493-
template1: &infrav1.Metal3DataTemplate{
494-
ObjectMeta: templateMeta,
495-
Spec: infrav1.Metal3DataTemplateSpec{},
496-
},
497-
indexes: map[int]string{},
498-
template2: &infrav1.Metal3DataTemplate{
499-
ObjectMeta: testObjectMeta("abc1", namespaceName, ""),
500-
Spec: infrav1.Metal3DataTemplateSpec{
501-
TemplateReference: "abc",
502-
},
503-
Status: infrav1.Metal3DataTemplateStatus{
504-
Indexes: map[string]int{},
505-
},
506-
},
507-
dataClaim: &infrav1.Metal3DataClaim{
508-
ObjectMeta: testObjectMetaWithOR(metal3DataClaimName, metal3machineName),
509-
},
510-
expectTemplateReference: true,
511-
}),
512-
Entry("TemplateReferenceDoNotExist", testCaseTemplateReference{
513-
template1: &infrav1.Metal3DataTemplate{
514-
ObjectMeta: templateMeta,
515-
Spec: infrav1.Metal3DataTemplateSpec{},
516-
},
517-
indexes: map[int]string{},
518-
template2: &infrav1.Metal3DataTemplate{
519-
ObjectMeta: testObjectMeta("abc1", namespaceName, ""),
520-
Spec: infrav1.Metal3DataTemplateSpec{},
521-
Status: infrav1.Metal3DataTemplateStatus{
522-
Indexes: map[string]int{},
523-
},
524-
},
525-
dataClaim: &infrav1.Metal3DataClaim{
526-
ObjectMeta: testObjectMetaWithOR(metal3DataClaimName, metal3machineName),
527-
},
528-
expectTemplateReference: false,
529-
}),
530-
Entry("TemplateReferenceRefersToOldTemplate", testCaseTemplateReference{
531-
template1: &infrav1.Metal3DataTemplate{
532-
ObjectMeta: metav1.ObjectMeta{
533-
Name: "template1",
534-
Namespace: namespaceName,
535-
},
536-
Spec: infrav1.Metal3DataTemplateSpec{},
537-
},
538-
indexes: map[int]string{},
539-
template2: &infrav1.Metal3DataTemplate{
540-
ObjectMeta: metav1.ObjectMeta{
541-
Name: "template2",
542-
Namespace: namespaceName,
543-
},
544-
Spec: infrav1.Metal3DataTemplateSpec{
545-
TemplateReference: "template1",
546-
},
547-
Status: infrav1.Metal3DataTemplateStatus{
548-
Indexes: map[string]int{},
549-
},
550-
},
551-
dataClaim: &infrav1.Metal3DataClaim{
552-
ObjectMeta: testObjectMetaWithOR(metal3DataClaimName, metal3machineName),
553-
},
554-
expectTemplateReference: true,
555-
expectDataObjectAssociated: true,
556-
}),
557-
Entry("TemplateReferenceRefersToZombieTemplate", testCaseTemplateReference{
558-
template1: &infrav1.Metal3DataTemplate{
559-
ObjectMeta: metav1.ObjectMeta{
560-
Name: "template1",
561-
Namespace: namespaceName,
562-
},
563-
Spec: infrav1.Metal3DataTemplateSpec{},
564-
},
565-
indexes: map[int]string{},
566-
template2: &infrav1.Metal3DataTemplate{
567-
ObjectMeta: metav1.ObjectMeta{
568-
Name: "template2",
569-
Namespace: namespaceName,
570-
},
571-
Spec: infrav1.Metal3DataTemplateSpec{
572-
TemplateReference: "template1",
573-
},
574-
Status: infrav1.Metal3DataTemplateStatus{
575-
Indexes: map[string]int{},
576-
},
577-
},
578-
dataClaim: &infrav1.Metal3DataClaim{
579-
ObjectMeta: testObjectMetaWithOR(metal3DataClaimName, metal3machineName),
580-
},
581-
dataObject: &infrav1.Metal3Data{
582-
ObjectMeta: testObjectMeta(metal3DataName, namespaceName, ""),
583-
Spec: infrav1.Metal3DataSpec{
584-
Index: 0,
585-
Template: corev1.ObjectReference{
586-
Name: "template12",
587-
},
588-
Claim: corev1.ObjectReference{
589-
Name: "abc",
590-
},
591-
},
592-
},
593-
expectDataObjectAssociated: false,
594-
}),
595-
)
596-
597429
type testCaseCreateAddresses struct {
598430
template *infrav1.Metal3DataTemplate
599431
dataClaim *infrav1.Metal3DataClaim

‎docs/api.md

-41
Original file line numberDiff line numberDiff line change
@@ -591,7 +591,6 @@ metadata:
591591
kind: Metal3Cluster
592592
name: cluster-1
593593
spec:
594-
templateReference: old-template
595594
metaData:
596595
strings:
597596
- key: abc
@@ -893,45 +892,6 @@ The object for the **services** section can be:
893892
- **dns**: a list of dns service with the ip address of a dns server
894893
- **dnsFromIPPool**: the IPPool from which to fetch the dns servers list
895894

896-
#### Updating metaData and networkData
897-
898-
The data template parts containing the metadata and networkData must be
899-
immutable since the BareMetalHost references the secrets and they are used at
900-
provisioning time, the secrets cannot be updated to be able to reprovision the
901-
node in the exact same state. This means that updates have to be done by
902-
creating a new template and referencing it in the new/updated
903-
Metal3MachineTemplate.
904-
905-
The process to allow updating the metaData and networkData is then to create a
906-
new Metal3DataTemplate and reference the new one in the Metal3MachineTemplate.
907-
This requires the Metal3Data to be linked to both Metal3DataTemplates. This is
908-
achieved using the `templateReference` field of the Metal3DataTemplate. When a
909-
Metal3Data corresponding to the Metal3DataTemplate is created, the reconciler
910-
will set the `templateReference` to the value of the Metal3DataTemplate, if set.
911-
912-
The Metal3Data objects are linked to a Metal3DataTemplate by three ways:
913-
914-
- Directly reference the template in the `template` field of the Metal3Data
915-
`spec`
916-
- They have the same `templateReference` key as the template
917-
- The template's `templateReference` matches the `name` of the `template` field
918-
of the `spec` of the Metal3Data.
919-
920-
The third way ensures backward compatibility with previous version of
921-
implementation when there was no `templateReference` in Metal3Data objects.
922-
Since the `templateReference` field is an addition to the existing API, the
923-
default behaviour of the controller will be to list the Metal3Data objects by
924-
matching their template field if the `templateReference` is left empty on the
925-
template object. However, if the `templateReference` is set on the
926-
Metal3DataTemplate object, but not on the Metal3Data object, then the controller
927-
will match the `templateReference` field with the `name` of the `template` field
928-
of the Metal3Data object. This is performed by setting the `templateReference`
929-
value on a new Metal3DataTemplate object to the name of an old
930-
Metal3DataTemplate object. This allows to transition from the old template
931-
object, which is without `templateReference` set on the Metal3Data objects
932-
created from the old template object to the new one which uses the
933-
`templateReference`.
934-
935895
## The Metal3DataClaim object
936896

937897
A new object would be created, a Metal3DataClaim type.
@@ -980,7 +940,6 @@ metadata:
980940
kind: Metal3DataTemplate
981941
name: nodepool-1
982942
spec:
983-
templateReference: old-template
984943
index: 0
985944
claim:
986945
name: machine-1

0 commit comments

Comments
 (0)