Skip to content

Commit 5fe5f8b

Browse files
shyamradhakrishnanshyamradhakrishnan
authored
Fix bug where clusterctl move does not work, and add correct default label (#74)
1 parent 9856a28 commit 5fe5f8b

32 files changed

+385
-164
lines changed

‎api/v1beta1/ocicluster_types.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,17 @@ const (
3232

3333
// OCIClusterSpec defines the desired state of OciCluster
3434
type OCIClusterSpec struct {
35+
36+
// The unique ID which will be used to tag all the resources created by this Cluster.
37+
// The tag will be used to identify resources belonging to this cluster.
38+
// this will be auto-generated and should not be set by the user.
39+
// +optional
40+
OCIResourceIdentifier string `json:"ociResourceIdentifier,omitempty"`
41+
3542
// NetworkSpec encapsulates all things related to OCI network.
3643
// +optional
3744
NetworkSpec NetworkSpec `json:"networkSpec,omitempty"`
45+
3846
// Free-form tags for this resource.
3947
// +optional
4048
FreeformTags map[string]string `json:"freeformTags,omitempty"`
@@ -101,6 +109,11 @@ func (c *OCICluster) SetConditions(conditions clusterv1.Conditions) {
101109
c.Status.Conditions = conditions
102110
}
103111

112+
// GetOCIResourceIdentifier will return the OCI resource identifier.
113+
func (c *OCICluster) GetOCIResourceIdentifier() string {
114+
return c.Spec.OCIResourceIdentifier
115+
}
116+
104117
func init() {
105118
SchemeBuilder.Register(&OCICluster{}, &OCIClusterList{})
106119
}

‎api/v1beta1/ocicluster_webhook.go

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,19 +24,27 @@ import (
2424

2525
apierrors "k8s.io/apimachinery/pkg/api/errors"
2626
"k8s.io/apimachinery/pkg/runtime"
27+
"k8s.io/apimachinery/pkg/util/uuid"
2728
"k8s.io/apimachinery/pkg/util/validation/field"
28-
2929
ctrl "sigs.k8s.io/controller-runtime"
3030
"sigs.k8s.io/controller-runtime/pkg/webhook"
3131
)
3232

3333
var clusterlogger = ctrl.Log.WithName("ocicluster-resource")
3434

3535
var (
36+
_ webhook.Defaulter = &OCICluster{}
3637
_ webhook.Validator = &OCICluster{}
3738
)
3839

3940
// +kubebuilder:webhook:verbs=create;update,path=/validate-infrastructure-cluster-x-k8s-io-v1beta1-ocicluster,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=ociclusters,versions=v1beta1,name=validation.ocicluster.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1
41+
// +kubebuilder:webhook:verbs=create;update,path=/mutate-infrastructure-cluster-x-k8s-io-v1beta1-ocicluster,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=ociclusters,versions=v1beta1,name=default.ocicluster.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1
42+
43+
func (c *OCICluster) Default() {
44+
if c.Spec.OCIResourceIdentifier == "" {
45+
c.Spec.OCIResourceIdentifier = string(uuid.NewUUID())
46+
}
47+
}
4048

4149
func (c *OCICluster) SetupWebhookWithManager(mgr ctrl.Manager) error {
4250
return ctrl.NewWebhookManagedBy(mgr).
@@ -81,6 +89,10 @@ func (c *OCICluster) ValidateUpdate(old runtime.Object) error {
8189
allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "region"), c.Spec.Region, "field is immutable"))
8290
}
8391

92+
if c.Spec.OCIResourceIdentifier != oldCluster.Spec.OCIResourceIdentifier {
93+
allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "ociResourceIdentifier"), c.Spec.OCIResourceIdentifier, "field is immutable"))
94+
}
95+
8496
allErrs = append(allErrs, c.validate()...)
8597

8698
if len(allErrs) == 0 {
@@ -107,6 +119,12 @@ func (c *OCICluster) validate() field.ErrorList {
107119
field.Invalid(field.NewPath("spec", "compartmentId"), c.Spec.CompartmentId, "field is invalid"))
108120
}
109121

122+
if len(c.Spec.OCIResourceIdentifier) <= 0 {
123+
allErrs = append(
124+
allErrs,
125+
field.Invalid(field.NewPath("spec", "ociResourceIdentifier"), c.Spec.OCIResourceIdentifier, "field is required"))
126+
}
127+
110128
if len(allErrs) == 0 {
111129
return nil
112130
}

‎api/v1beta1/ocicluster_webhook_test.go

Lines changed: 65 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"testing"
2424

2525
"github.com/onsi/gomega"
26+
. "github.com/onsi/gomega"
2627
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2728
)
2829

@@ -52,13 +53,24 @@ func TestOCICluster_ValidateCreate(t *testing.T) {
5253
expectErr: true,
5354
},
5455
{
55-
name: "should succeed",
56+
name: "shouldn't allow blank OCIResourceIdentifier",
5657
c: &OCICluster{
5758
ObjectMeta: metav1.ObjectMeta{},
5859
Spec: OCIClusterSpec{
5960
CompartmentId: "ocid",
6061
},
6162
},
63+
expectErr: true,
64+
},
65+
{
66+
name: "should succeed",
67+
c: &OCICluster{
68+
ObjectMeta: metav1.ObjectMeta{},
69+
Spec: OCIClusterSpec{
70+
CompartmentId: "ocid",
71+
OCIResourceIdentifier: "uuid",
72+
},
73+
},
6274
expectErr: false,
6375
},
6476
}
@@ -98,19 +110,40 @@ func TestOCICluster_ValidateUpdate(t *testing.T) {
98110
},
99111
expectErr: true,
100112
},
113+
{
114+
name: "shouldn't change OCIResourceIdentifier",
115+
c: &OCICluster{
116+
ObjectMeta: metav1.ObjectMeta{},
117+
Spec: OCIClusterSpec{
118+
CompartmentId: "ocid",
119+
Region: "old-region",
120+
OCIResourceIdentifier: "uuid-1",
121+
},
122+
},
123+
old: &OCICluster{
124+
ObjectMeta: metav1.ObjectMeta{},
125+
Spec: OCIClusterSpec{
126+
Region: "old-region",
127+
OCIResourceIdentifier: "uuid-2",
128+
},
129+
},
130+
expectErr: true,
131+
},
101132
{
102133
name: "should succeed",
103134
c: &OCICluster{
104135
ObjectMeta: metav1.ObjectMeta{},
105136
Spec: OCIClusterSpec{
106-
CompartmentId: "ocid",
107-
Region: "old-region",
137+
CompartmentId: "ocid",
138+
Region: "old-region",
139+
OCIResourceIdentifier: "uuid",
108140
},
109141
},
110142
old: &OCICluster{
111143
ObjectMeta: metav1.ObjectMeta{},
112144
Spec: OCIClusterSpec{
113-
Region: "old-region",
145+
Region: "old-region",
146+
OCIResourceIdentifier: "uuid",
114147
},
115148
},
116149
expectErr: false,
@@ -128,5 +161,33 @@ func TestOCICluster_ValidateUpdate(t *testing.T) {
128161
}
129162
})
130163
}
164+
}
165+
166+
func TestOCICluster_CreateDefault(t *testing.T) {
131167

168+
tests := []struct {
169+
name string
170+
c *OCICluster
171+
expect func(g *gomega.WithT, c *OCICluster)
172+
}{
173+
{
174+
name: "should set default OCIResourceIdentifier",
175+
c: &OCICluster{
176+
ObjectMeta: metav1.ObjectMeta{},
177+
Spec: OCIClusterSpec{
178+
CompartmentId: "badocid",
179+
},
180+
},
181+
expect: func(g *gomega.WithT, c *OCICluster) {
182+
g.Expect(c.Spec.OCIResourceIdentifier).To(Not(BeNil()))
183+
},
184+
},
185+
}
186+
for _, test := range tests {
187+
t.Run(test.name, func(t *testing.T) {
188+
g := gomega.NewWithT(t)
189+
test.c.Default()
190+
test.expect(g, test.c)
191+
})
192+
}
132193
}

‎cloud/ociutil/ociutil.go

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,11 @@ package ociutil
1919
import (
2020
"context"
2121
"fmt"
22-
nlb "github.com/oracle/cluster-api-provider-oci/cloud/services/networkloadbalancer"
2322
"net/http"
2423
"time"
2524

25+
nlb "github.com/oracle/cluster-api-provider-oci/cloud/services/networkloadbalancer"
26+
2627
"github.com/oracle/oci-go-sdk/v63/common"
2728
"github.com/oracle/oci-go-sdk/v63/core"
2829
"github.com/oracle/oci-go-sdk/v63/networkloadbalancer"
@@ -31,9 +32,12 @@ import (
3132
)
3233

3334
const (
34-
WorkRequestPollInterval = 5 * time.Second
35-
WorkRequestTimeout = 2 * time.Minute
36-
MaxOPCRetryTokenBytes = 64
35+
WorkRequestPollInterval = 5 * time.Second
36+
WorkRequestTimeout = 2 * time.Minute
37+
MaxOPCRetryTokenBytes = 64
38+
CreatedBy = "CreatedBy"
39+
OCIClusterAPIProvider = "OCIClusterAPIProvider"
40+
ClusterResourceIdentifier = "ClusterResourceIdentifier"
3741
)
3842

3943
// ErrNotFound is for simulation during testing, OCI SDK does not have a way
@@ -102,13 +106,13 @@ func GetBaseLineOcpuOptimizationEnum(baseLineOcpuOptmimizationString string) (co
102106
// GetDefaultClusterTags creates and returns a map of the default tags for all clusters
103107
func GetDefaultClusterTags() map[string]string {
104108
tags := make(map[string]string)
105-
tags["CreatedBy"] = "OCIClusterAPIProvider"
109+
tags[CreatedBy] = OCIClusterAPIProvider
106110
return tags
107111
}
108112

109-
// BuildClusterTags uses the default tags and adds the ClusterUUID tag
110-
func BuildClusterTags(clusterUUID string) map[string]string {
113+
// BuildClusterTags uses the default tags and adds the ClusterResourceUID tag
114+
func BuildClusterTags(ClusterResourceUID string) map[string]string {
111115
tags := GetDefaultClusterTags()
112-
tags["ClusterUUID"] = clusterUUID
116+
tags[ClusterResourceIdentifier] = ClusterResourceUID
113117
return tags
114118
}

‎cloud/ociutil/ociutil_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,8 @@ func TestGetCloudProviderConfig(t *testing.T) {
7171
func TestAddToDefaultClusterTags(t *testing.T) {
7272
testUUID := "UUIDTEST"
7373
tags := BuildClusterTags(testUUID)
74-
if tags["ClusterUUID"] != testUUID {
75-
t.Errorf("Tags don't match Expected: %s, Actual: %s", testUUID, tags["ClusterUUID"])
74+
if tags[ClusterResourceIdentifier] != testUUID {
75+
t.Errorf("Tags don't match Expected: %s, Actual: %s", testUUID, tags[ClusterResourceIdentifier])
7676
}
7777

7878
// should also contain default tags

‎cloud/scope/cluster.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ func (s *ClusterScope) ReconcileFailureDomains(ctx context.Context) error {
124124
}
125125

126126
func (s *ClusterScope) IsResourceCreatedByClusterAPI(resourceFreeFormTags map[string]string) bool {
127-
tagsAddedByClusterAPI := ociutil.BuildClusterTags(string(s.OCICluster.UID))
127+
tagsAddedByClusterAPI := ociutil.BuildClusterTags(s.OCICluster.GetOCIResourceIdentifier())
128128
for k, v := range tagsAddedByClusterAPI {
129129
if resourceFreeFormTags[k] != v {
130130
return false
@@ -251,7 +251,7 @@ func (s *ClusterScope) GetFreeFormTags() map[string]string {
251251
if tags == nil {
252252
tags = make(map[string]string)
253253
}
254-
tagsAddedByClusterAPI := ociutil.BuildClusterTags(string(s.OCICluster.UID))
254+
tagsAddedByClusterAPI := ociutil.BuildClusterTags(string(s.OCICluster.GetOCIResourceIdentifier()))
255255
for k, v := range tagsAddedByClusterAPI {
256256
tags[k] = v
257257
}

‎cloud/scope/drg_reconciler.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ func (s *ClusterScope) createDRG(ctx context.Context) (*core.Drg, error) {
123123
DefinedTags: s.GetDefinedTags(),
124124
DisplayName: common.String(s.GetDRGName()),
125125
},
126-
OpcRetryToken: ociutil.GetOPCRetryToken("%s-%s", "create-drg", string(s.OCICluster.UID)),
126+
OpcRetryToken: ociutil.GetOPCRetryToken("%s-%s", "create-drg", string(s.OCICluster.GetOCIResourceIdentifier())),
127127
})
128128
if err != nil {
129129
return nil, err

‎cloud/scope/drg_reconciler_test.go

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,12 @@ func TestDRGReconciliation(t *testing.T) {
5151
client := fake.NewClientBuilder().Build()
5252
ociCluster = infrastructurev1beta1.OCICluster{
5353
ObjectMeta: metav1.ObjectMeta{
54-
UID: "a",
54+
UID: "cluster_uid",
5555
Name: "cluster",
5656
},
5757
Spec: infrastructurev1beta1.OCIClusterSpec{
58-
CompartmentId: "compartment-id",
58+
CompartmentId: "compartment-id",
59+
OCIResourceIdentifier: "resource_uid",
5960
},
6061
}
6162
ociCluster.Spec.ControlPlaneEndpoint.Port = 6443
@@ -66,8 +67,8 @@ func TestDRGReconciliation(t *testing.T) {
6667
Client: client,
6768
})
6869
tags = make(map[string]string)
69-
tags["CreatedBy"] = "OCIClusterAPIProvider"
70-
tags["ClusterUUID"] = "a"
70+
tags[ociutil.CreatedBy] = ociutil.OCIClusterAPIProvider
71+
tags[ociutil.ClusterResourceIdentifier] = "resource_uid"
7172
vcnPeering = infrastructurev1beta1.VCNPeering{}
7273
g.Expect(err).To(BeNil())
7374
}
@@ -167,8 +168,8 @@ func TestDRGReconciliation(t *testing.T) {
167168
vcnPeering.DRG.Manage = true
168169
vcnPeering.DRG.ID = common.String("drg-id")
169170
existingTags := make(map[string]string)
170-
existingTags["CreatedBy"] = "OCIClusterAPIProvider"
171-
existingTags["ClusterUUID"] = "a"
171+
existingTags[ociutil.CreatedBy] = ociutil.OCIClusterAPIProvider
172+
existingTags[ociutil.ClusterResourceIdentifier] = "resource_uid"
172173
existingTags["test"] = "test"
173174
clusterScope.OCICluster.Spec.NetworkSpec.VCNPeering = &vcnPeering
174175
vcnClient.EXPECT().GetDrg(gomock.Any(), gomock.Eq(core.GetDrgRequest{
@@ -209,7 +210,7 @@ func TestDRGReconciliation(t *testing.T) {
209210
DefinedTags: make(map[string]map[string]interface{}),
210211
DisplayName: common.String("cluster"),
211212
},
212-
OpcRetryToken: ociutil.GetOPCRetryToken("%s-%s", "create-drg", "a"),
213+
OpcRetryToken: ociutil.GetOPCRetryToken("%s-%s", "create-drg", "resource_uid"),
213214
})).
214215
Return(core.CreateDrgResponse{
215216
Drg: core.Drg{
@@ -258,11 +259,12 @@ func TestDRGDeletion(t *testing.T) {
258259
client := fake.NewClientBuilder().Build()
259260
ociCluster = infrastructurev1beta1.OCICluster{
260261
ObjectMeta: metav1.ObjectMeta{
261-
UID: "a",
262+
UID: "cluster_uid",
262263
Name: "cluster",
263264
},
264265
Spec: infrastructurev1beta1.OCIClusterSpec{
265-
CompartmentId: "compartment-id",
266+
CompartmentId: "compartment-id",
267+
OCIResourceIdentifier: "resource_uid",
266268
},
267269
}
268270
ociCluster.Spec.ControlPlaneEndpoint.Port = 6443
@@ -273,8 +275,8 @@ func TestDRGDeletion(t *testing.T) {
273275
Client: client,
274276
})
275277
tags = make(map[string]string)
276-
tags["CreatedBy"] = "OCIClusterAPIProvider"
277-
tags["ClusterUUID"] = "a"
278+
tags[ociutil.CreatedBy] = ociutil.OCIClusterAPIProvider
279+
tags[ociutil.ClusterResourceIdentifier] = "resource_uid"
278280
vcnPeering = infrastructurev1beta1.VCNPeering{}
279281
g.Expect(err).To(BeNil())
280282
}

0 commit comments

Comments
 (0)