Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

⚠️ Remove metal3datatemplate template reference #2265

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions api/v1beta1/metal3data_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,6 @@ type Metal3DataSpec struct {
// +optional
Index int `json:"index,omitempty"`

// 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 points to the rendered MetaData secret.
// +optional
MetaData *corev1.SecretReference `json:"metaData,omitempty"`
Expand Down
3 changes: 1 addition & 2 deletions api/v1beta1/metal3data_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down
6 changes: 0 additions & 6 deletions api/v1beta1/metal3datatemplate_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
49 changes: 6 additions & 43 deletions baremetal/metal3datatemplate_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
168 changes: 0 additions & 168 deletions baremetal/metal3datatemplate_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading