Fix bug where clusterctl move does not work, and add correct default …

…label

api/v1beta1/ocicluster_types.go

@@ -32,9 +32,17 @@ const (
 
 // OCIClusterSpec defines the desired state of OciCluster
 type OCIClusterSpec struct {
+
+	// The unique ID which will be used to tag all the resources created by this Cluster.
+	// The tag will be used to identify resources belonging to this cluster.
+	// this will be auto-generated and should not be set by the user.
+	// +optional
+	OCIResourceIdentifier string `json:"ociResourceIdentifier,omitempty"`
+
 	// NetworkSpec encapsulates all things related to OCI network.
 	// +optional
 	NetworkSpec NetworkSpec `json:"networkSpec,omitempty"`
+
 	// Free-form tags for this resource.
 	// +optional
 	FreeformTags map[string]string `json:"freeformTags,omitempty"`
@@ -101,6 +109,11 @@ func (c *OCICluster) SetConditions(conditions clusterv1.Conditions) {
 	c.Status.Conditions = conditions
 }
 
+// GetOCIResourceIdentifier will return the OCI resource identifier.
+func (c *OCICluster) GetOCIResourceIdentifier() string {
+	return c.Spec.OCIResourceIdentifier
+}
+
 func init() {
 	SchemeBuilder.Register(&OCICluster{}, &OCIClusterList{})
 }

api/v1beta1/ocicluster_webhook.go

@@ -24,19 +24,27 @@ import (
 
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/uuid"
 	"k8s.io/apimachinery/pkg/util/validation/field"
-
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 )
 
 var clusterlogger = ctrl.Log.WithName("ocicluster-resource")
 
 var (
+	_ webhook.Defaulter = &OCICluster{}
 	_ webhook.Validator = &OCICluster{}
 )
 
 // +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
+// +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
+
+func (c *OCICluster) Default() {
+	if c.Spec.OCIResourceIdentifier == "" {
+		c.Spec.OCIResourceIdentifier = string(uuid.NewUUID())
+	}
+}
 
 func (c *OCICluster) SetupWebhookWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewWebhookManagedBy(mgr).
@@ -81,6 +89,10 @@ func (c *OCICluster) ValidateUpdate(old runtime.Object) error {
 		allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "region"), c.Spec.Region, "field is immutable"))
 	}
 
+	if c.Spec.OCIResourceIdentifier != oldCluster.Spec.OCIResourceIdentifier {
+		allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "ociResourceIdentifier"), c.Spec.OCIResourceIdentifier, "field is immutable"))
+	}
+
 	allErrs = append(allErrs, c.validate()...)
 
 	if len(allErrs) == 0 {
@@ -107,6 +119,12 @@ func (c *OCICluster) validate() field.ErrorList {
 			field.Invalid(field.NewPath("spec", "compartmentId"), c.Spec.CompartmentId, "field is invalid"))
 	}
 
+	if len(c.Spec.OCIResourceIdentifier) <= 0 {
+		allErrs = append(
+			allErrs,
+			field.Invalid(field.NewPath("spec", "ociResourceIdentifier"), c.Spec.OCIResourceIdentifier, "field is required"))
+	}
+
 	if len(allErrs) == 0 {
 		return nil
 	}

api/v1beta1/ocicluster_webhook_test.go

@@ -23,6 +23,7 @@ import (
 	"testing"
 
 	"github.com/onsi/gomega"
+	. "github.com/onsi/gomega"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -52,13 +53,24 @@ func TestOCICluster_ValidateCreate(t *testing.T) {
 			expectErr: true,
 		},
 		{
-			name: "should succeed",
+			name: "shouldn't allow blank OCIResourceIdentifier",
 			c: &OCICluster{
 				ObjectMeta: metav1.ObjectMeta{},
 				Spec: OCIClusterSpec{
 					CompartmentId: "ocid",
 				},
 			},
+			expectErr: true,
+		},
+		{
+			name: "should succeed",
+			c: &OCICluster{
+				ObjectMeta: metav1.ObjectMeta{},
+				Spec: OCIClusterSpec{
+					CompartmentId:         "ocid",
+					OCIResourceIdentifier: "uuid",
+				},
+			},
 			expectErr: false,
 		},
 	}
@@ -98,19 +110,40 @@ func TestOCICluster_ValidateUpdate(t *testing.T) {
 			},
 			expectErr: true,
 		},
+		{
+			name: "shouldn't change OCIResourceIdentifier",
+			c: &OCICluster{
+				ObjectMeta: metav1.ObjectMeta{},
+				Spec: OCIClusterSpec{
+					CompartmentId:         "ocid",
+					Region:                "old-region",
+					OCIResourceIdentifier: "uuid-1",
+				},
+			},
+			old: &OCICluster{
+				ObjectMeta: metav1.ObjectMeta{},
+				Spec: OCIClusterSpec{
+					Region:                "old-region",
+					OCIResourceIdentifier: "uuid-2",
+				},
+			},
+			expectErr: true,
+		},
 		{
 			name: "should succeed",
 			c: &OCICluster{
 				ObjectMeta: metav1.ObjectMeta{},
 				Spec: OCIClusterSpec{
-					CompartmentId: "ocid",
-					Region:        "old-region",
+					CompartmentId:         "ocid",
+					Region:                "old-region",
+					OCIResourceIdentifier: "uuid",
 				},
 			},
 			old: &OCICluster{
 				ObjectMeta: metav1.ObjectMeta{},
 				Spec: OCIClusterSpec{
-					Region: "old-region",
+					Region:                "old-region",
+					OCIResourceIdentifier: "uuid",
 				},
 			},
 			expectErr: false,
@@ -128,5 +161,33 @@ func TestOCICluster_ValidateUpdate(t *testing.T) {
 			}
 		})
 	}
+}
+
+func TestOCICluster_CreateDefault(t *testing.T) {
 
+	tests := []struct {
+		name   string
+		c      *OCICluster
+		expect func(g *gomega.WithT, c *OCICluster)
+	}{
+		{
+			name: "should set default OCIResourceIdentifier",
+			c: &OCICluster{
+				ObjectMeta: metav1.ObjectMeta{},
+				Spec: OCIClusterSpec{
+					CompartmentId: "badocid",
+				},
+			},
+			expect: func(g *gomega.WithT, c *OCICluster) {
+				g.Expect(c.Spec.OCIResourceIdentifier).To(Not(BeNil()))
+			},
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			g := gomega.NewWithT(t)
+			test.c.Default()
+			test.expect(g, test.c)
+		})
+	}
 }

cloud/ociutil/ociutil.go

@@ -19,10 +19,11 @@ package ociutil
 import (
 	"context"
 	"fmt"
-	nlb "github.com/oracle/cluster-api-provider-oci/cloud/services/networkloadbalancer"
 	"net/http"
 	"time"
 
+	nlb "github.com/oracle/cluster-api-provider-oci/cloud/services/networkloadbalancer"
+
 	"github.com/oracle/oci-go-sdk/v63/common"
 	"github.com/oracle/oci-go-sdk/v63/core"
 	"github.com/oracle/oci-go-sdk/v63/networkloadbalancer"
@@ -31,9 +32,12 @@ import (
 )
 
 const (
-	WorkRequestPollInterval = 5 * time.Second
-	WorkRequestTimeout      = 2 * time.Minute
-	MaxOPCRetryTokenBytes   = 64
+	WorkRequestPollInterval   = 5 * time.Second
+	WorkRequestTimeout        = 2 * time.Minute
+	MaxOPCRetryTokenBytes     = 64
+	CreatedBy                 = "CreatedBy"
+	OCIClusterAPIProvider     = "OCIClusterAPIProvider"
+	ClusterResourceIdentifier = "ClusterResourceIdentifier"
 )
 
 // ErrNotFound is for simulation during testing, OCI SDK does not have a way
@@ -102,13 +106,13 @@ func GetBaseLineOcpuOptimizationEnum(baseLineOcpuOptmimizationString string) (co
 // GetDefaultClusterTags creates and returns a map of the default tags for all clusters
 func GetDefaultClusterTags() map[string]string {
 	tags := make(map[string]string)
-	tags["CreatedBy"] = "OCIClusterAPIProvider"
+	tags[CreatedBy] = OCIClusterAPIProvider
 	return tags
 }
 
-// BuildClusterTags uses the default tags and adds the ClusterUUID tag
-func BuildClusterTags(clusterUUID string) map[string]string {
+// BuildClusterTags uses the default tags and adds the ClusterResourceUID tag
+func BuildClusterTags(ClusterResourceUID string) map[string]string {
 	tags := GetDefaultClusterTags()
-	tags["ClusterUUID"] = clusterUUID
+	tags[ClusterResourceIdentifier] = ClusterResourceUID
 	return tags
 }

cloud/ociutil/ociutil_test.go

@@ -71,8 +71,8 @@ func TestGetCloudProviderConfig(t *testing.T) {
 func TestAddToDefaultClusterTags(t *testing.T) {
 	testUUID := "UUIDTEST"
 	tags := BuildClusterTags(testUUID)
-	if tags["ClusterUUID"] != testUUID {
-		t.Errorf("Tags don't match Expected: %s, Actual: %s", testUUID, tags["ClusterUUID"])
+	if tags[ClusterResourceIdentifier] != testUUID {
+		t.Errorf("Tags don't match Expected: %s, Actual: %s", testUUID, tags[ClusterResourceIdentifier])
 	}
 
 	// should also contain default tags

cloud/scope/cluster.go

@@ -124,7 +124,7 @@ func (s *ClusterScope) ReconcileFailureDomains(ctx context.Context) error {
 }
 
 func (s *ClusterScope) IsResourceCreatedByClusterAPI(resourceFreeFormTags map[string]string) bool {
-	tagsAddedByClusterAPI := ociutil.BuildClusterTags(string(s.OCICluster.UID))
+	tagsAddedByClusterAPI := ociutil.BuildClusterTags(s.OCICluster.GetOCIResourceIdentifier())
 	for k, v := range tagsAddedByClusterAPI {
 		if resourceFreeFormTags[k] != v {
 			return false
@@ -251,7 +251,7 @@ func (s *ClusterScope) GetFreeFormTags() map[string]string {
 	if tags == nil {
 		tags = make(map[string]string)
 	}
-	tagsAddedByClusterAPI := ociutil.BuildClusterTags(string(s.OCICluster.UID))
+	tagsAddedByClusterAPI := ociutil.BuildClusterTags(string(s.OCICluster.GetOCIResourceIdentifier()))
 	for k, v := range tagsAddedByClusterAPI {
 		tags[k] = v
 	}

cloud/scope/drg_reconciler.go

@@ -123,7 +123,7 @@ func (s *ClusterScope) createDRG(ctx context.Context) (*core.Drg, error) {
 			DefinedTags:   s.GetDefinedTags(),
 			DisplayName:   common.String(s.GetDRGName()),
 		},
-		OpcRetryToken: ociutil.GetOPCRetryToken("%s-%s", "create-drg", string(s.OCICluster.UID)),
+		OpcRetryToken: ociutil.GetOPCRetryToken("%s-%s", "create-drg", string(s.OCICluster.GetOCIResourceIdentifier())),
 	})
 	if err != nil {
 		return nil, err

cloud/scope/drg_reconciler_test.go

@@ -51,11 +51,12 @@ func TestDRGReconciliation(t *testing.T) {
 		client := fake.NewClientBuilder().Build()
 		ociCluster = infrastructurev1beta1.OCICluster{
 			ObjectMeta: metav1.ObjectMeta{
-				UID:  "a",
+				UID:  "cluster_uid",
 				Name: "cluster",
 			},
 			Spec: infrastructurev1beta1.OCIClusterSpec{
-				CompartmentId: "compartment-id",
+				CompartmentId:         "compartment-id",
+				OCIResourceIdentifier: "resource_uid",
 			},
 		}
 		ociCluster.Spec.ControlPlaneEndpoint.Port = 6443
@@ -66,8 +67,8 @@ func TestDRGReconciliation(t *testing.T) {
 			Client:     client,
 		})
 		tags = make(map[string]string)
-		tags["CreatedBy"] = "OCIClusterAPIProvider"
-		tags["ClusterUUID"] = "a"
+		tags[ociutil.CreatedBy] = ociutil.OCIClusterAPIProvider
+		tags[ociutil.ClusterResourceIdentifier] = "resource_uid"
 		vcnPeering = infrastructurev1beta1.VCNPeering{}
 		g.Expect(err).To(BeNil())
 	}
@@ -167,8 +168,8 @@ func TestDRGReconciliation(t *testing.T) {
 				vcnPeering.DRG.Manage = true
 				vcnPeering.DRG.ID = common.String("drg-id")
 				existingTags := make(map[string]string)
-				existingTags["CreatedBy"] = "OCIClusterAPIProvider"
-				existingTags["ClusterUUID"] = "a"
+				existingTags[ociutil.CreatedBy] = ociutil.OCIClusterAPIProvider
+				existingTags[ociutil.ClusterResourceIdentifier] = "resource_uid"
 				existingTags["test"] = "test"
 				clusterScope.OCICluster.Spec.NetworkSpec.VCNPeering = &vcnPeering
 				vcnClient.EXPECT().GetDrg(gomock.Any(), gomock.Eq(core.GetDrgRequest{
@@ -209,7 +210,7 @@ func TestDRGReconciliation(t *testing.T) {
 						DefinedTags:   make(map[string]map[string]interface{}),
 						DisplayName:   common.String("cluster"),
 					},
-					OpcRetryToken: ociutil.GetOPCRetryToken("%s-%s", "create-drg", "a"),
+					OpcRetryToken: ociutil.GetOPCRetryToken("%s-%s", "create-drg", "resource_uid"),
 				})).
 					Return(core.CreateDrgResponse{
 						Drg: core.Drg{
@@ -258,11 +259,12 @@ func TestDRGDeletion(t *testing.T) {
 		client := fake.NewClientBuilder().Build()
 		ociCluster = infrastructurev1beta1.OCICluster{
 			ObjectMeta: metav1.ObjectMeta{
-				UID:  "a",
+				UID:  "cluster_uid",
 				Name: "cluster",
 			},
 			Spec: infrastructurev1beta1.OCIClusterSpec{
-				CompartmentId: "compartment-id",
+				CompartmentId:         "compartment-id",
+				OCIResourceIdentifier: "resource_uid",
 			},
 		}
 		ociCluster.Spec.ControlPlaneEndpoint.Port = 6443
@@ -273,8 +275,8 @@ func TestDRGDeletion(t *testing.T) {
 			Client:     client,
 		})
 		tags = make(map[string]string)
-		tags["CreatedBy"] = "OCIClusterAPIProvider"
-		tags["ClusterUUID"] = "a"
+		tags[ociutil.CreatedBy] = ociutil.OCIClusterAPIProvider
+		tags[ociutil.ClusterResourceIdentifier] = "resource_uid"
 		vcnPeering = infrastructurev1beta1.VCNPeering{}
 		g.Expect(err).To(BeNil())
 	}