fix cos flow

Signed-off-by: Prajyot-Parab <prajyot.parab2@ibm.com>

api/v1beta2/ibmpowervscluster_webhook.go

@@ -137,19 +137,5 @@ func (r *IBMPowerVSCluster) validateIBMPowerVSClusterCreateInfraPrereq() *field.
 		return nil
 	}
 
-	// TODO(Phase 1): If ignition is set and these resources are not set, auto create them.
-	// If ignition is set, make sure to check that CosInstanceName, BucketName and region is set
-	if r.Spec.CosInstance == nil {
-		return field.Invalid(field.NewPath("spec.cosInstance"), r.Spec.CosInstance, "ignition is set but value of cosInstance is empty")
-	}
-	if r.Spec.CosInstance.Name == "" {
-		return field.Invalid(field.NewPath("spec.cosInstance.name"), r.Spec.CosInstance, "ignition is set but value of cosInstance name is empty")
-	}
-	if r.Spec.CosInstance.BucketName == "" {
-		return field.Invalid(field.NewPath("spec.cosInstance.bucketName"), r.Spec.CosInstance, "ignition is set but value of bucketName is empty")
-	}
-	if r.Spec.CosInstance.BucketRegion == "" {
-		return field.Invalid(field.NewPath("spec.cosInstance.bucketRegion"), r.Spec.CosInstance, "ignition is set but value of bucketRegion is empty")
-	}
 	return nil
 }

api/v1beta2/types.go

@@ -136,6 +136,8 @@ var (
 	ResourceTypeSubnet = ResourceType("subnet")
 	// ResourceTypeCOSInstance is IBM COS instance resource.
 	ResourceTypeCOSInstance = ResourceType("cosInstance")
+	// ResourceTypeCOSBucket is IBM COS bucket resource.
+	ResourceTypeCOSBucket = ResourceType("cosBucket")
 	// ResourceTypeResourceGroup is IBM Resource Group.
 	ResourceTypeResourceGroup = ResourceType("resourceGroup")
 )

cloud/scope/powervs_cluster.go

@@ -1421,10 +1421,6 @@ func (s *PowerVSClusterScope) COSInstance() *infrav1beta2.CosInstance {
 
 // ReconcileCOSInstance reconcile COS bucket.
 func (s *PowerVSClusterScope) ReconcileCOSInstance() error {
-	if s.COSInstance() == nil || s.COSInstance().Name == "" {
-		return nil
-	}
-
 	// check COS service instance exist in cloud
 	cosServiceInstanceStatus, err := s.checkCOSServiceInstance()
 	if err != nil {
@@ -1453,14 +1449,10 @@ func (s *PowerVSClusterScope) ReconcileCOSInstance() error {
 	if !ok {
 		return fmt.Errorf("ibmcloud api key is not provided, set %s environmental variable", "IBMCLOUD_API_KEY")
 	}
-	region := s.IBMPowerVSCluster.Spec.CosInstance.BucketRegion
-	// if the bucket region is not set, use vpc region
+
+	region := s.bucketRegion()
 	if region == "" {
-		vpcDetails := s.VPC()
-		if vpcDetails == nil || vpcDetails.Region == nil {
-			return fmt.Errorf("failed to determine cos bucket region, both buckeet region and vpc region not set")
-		}
-		region = *vpcDetails.Region
+		return fmt.Errorf("failed to determine cos bucket region, both buckeet region and vpc region not set")
 	}
 
 	serviceEndpoint := fmt.Sprintf("s3.%s.%s", region, cosURLDomain)
@@ -1504,7 +1496,7 @@ func (s *PowerVSClusterScope) ReconcileCOSInstance() error {
 }
 
 func (s *PowerVSClusterScope) checkCOSBucket() (bool, error) {
-	if _, err := s.COSClient.GetBucketByName(s.COSInstance().BucketName); err != nil {
+	if _, err := s.COSClient.GetBucketByName(*s.GetServiceName(infrav1beta2.ResourceTypeCOSBucket)); err != nil {
 		if aerr, ok := err.(awserr.Error); ok {
 			switch aerr.Code() {
 			case s3.ErrCodeNoSuchBucket, "Forbidden", "NotFound":
@@ -1522,7 +1514,7 @@ func (s *PowerVSClusterScope) checkCOSBucket() (bool, error) {
 
 func (s *PowerVSClusterScope) createCOSBucket() error {
 	input := &s3.CreateBucketInput{
-		Bucket: pointer.String(s.COSInstance().BucketName),
+		Bucket: pointer.String(*s.GetServiceName(infrav1beta2.ResourceTypeCOSBucket)),
 	}
 	_, err := s.COSClient.CreateBucket(input)
 	if err == nil {
@@ -1547,12 +1539,12 @@ func (s *PowerVSClusterScope) createCOSBucket() error {
 
 func (s *PowerVSClusterScope) checkCOSServiceInstance() (*resourcecontrollerv2.ResourceInstance, error) {
 	// check cos service instance
-	serviceInstance, err := s.ResourceClient.GetInstanceByName(s.COSInstance().Name, resourcecontroller.CosResourceID, resourcecontroller.CosResourcePlanID)
+	serviceInstance, err := s.ResourceClient.GetInstanceByName(*s.GetServiceName(infrav1beta2.ResourceTypeCOSInstance), resourcecontroller.CosResourceID, resourcecontroller.CosResourcePlanID)
 	if err != nil {
 		return nil, err
 	}
 	if serviceInstance == nil {
-		s.Info("cos service instance is nil", "name", s.COSInstance().Name)
+		s.Info("cos service instance is nil", "name", *s.GetServiceName(infrav1beta2.ResourceTypeCOSInstance))
 		return nil, nil
 	}
 	if *serviceInstance.State != string(infrav1beta2.ServiceInstanceStateActive) {
@@ -1693,6 +1685,11 @@ func (s *PowerVSClusterScope) GetServiceName(resourceType infrav1beta2.ResourceT
 			return pointer.String(fmt.Sprintf("%s-cosinstance", s.InfraCluster()))
 		}
 		return &s.COSInstance().Name
+	case infrav1beta2.ResourceTypeCOSBucket:
+		if s.COSInstance() == nil || s.COSInstance().BucketName == "" {
+			return pointer.String(fmt.Sprintf("%s-cosbucket", s.InfraCluster()))
+		}
+		return &s.COSInstance().BucketName
 	case infrav1beta2.ResourceTypeSubnet:
 		return pointer.String(fmt.Sprintf("%s-vpcsubnet", s.InfraCluster()))
 	case infrav1beta2.ResourceTypeLoadBalancer:
@@ -1977,3 +1974,19 @@ func (s *PowerVSClusterScope) isResourceCreatedByController(resourceType infrav1
 	}
 	return false
 }
+
+// TODO: duplicate function, optimize it.
+func (s *PowerVSClusterScope) bucketRegion() string {
+	var region string
+	if s.COSInstance() != nil && s.COSInstance().BucketRegion != "" {
+		return s.COSInstance().BucketRegion
+	}
+	// if the bucket region is not set, use vpc region
+	if region == "" {
+		vpcDetails := s.VPC()
+		if vpcDetails != nil && vpcDetails.Region != nil {
+			return *vpcDetails.Region
+		}
+	}
+	return region
+}

cloud/scope/powervs_machine.go

@@ -408,7 +408,12 @@ func (m *PowerVSMachineScope) createIgnitionData(data []byte) (string, error) {
 	key := m.bootstrapDataKey()
 	m.Info("bootstrap data key", "key", key)
 
-	bucket := m.IBMPowerVSCluster.Spec.CosInstance.BucketName
+	bucket := m.bucketName()
+	region := m.bucketRegion()
+	if region == "" {
+		return "", fmt.Errorf("failed to determine cos bucket region, both bucket region and vpc region not set")
+	}
+
 	if _, err := cosClient.PutObject(&s3.PutObjectInput{
 		Body:   aws.ReadSeekCloser(bytes.NewReader(data)),
 		Bucket: aws.String(bucket),
@@ -418,8 +423,7 @@ func (m *PowerVSMachineScope) createIgnitionData(data []byte) (string, error) {
 		return "", fmt.Errorf("putting object to cos bucket %w", err)
 	}
 
-	bucketRegion := m.IBMPowerVSCluster.Spec.CosInstance.BucketRegion
-	objHost := fmt.Sprintf("%s.s3.%s.%s", bucket, bucketRegion, cosURLDomain)
+	objHost := fmt.Sprintf("%s.s3.%s.%s", bucket, region, cosURLDomain)
 	objectURL := &url.URL{
 		Scheme: "https",
 		Host:   objHost,
@@ -538,7 +542,7 @@ func (m *PowerVSMachineScope) DeleteMachineIgnition() error {
 		return fmt.Errorf("failed to create cosClient %w", err)
 	}
 
-	bucket := m.IBMPowerVSCluster.Spec.CosInstance.BucketName
+	bucket := m.bucketName()
 	objs, _ := cosClient.ListObjects(&s3.ListObjectsInput{
 		Bucket: aws.String(bucket),
 	})
@@ -561,10 +565,13 @@ func (m *PowerVSMachineScope) DeleteMachineIgnition() error {
 
 // createCOSClient creates a new cosClient from the supplied parameters.
 func (m *PowerVSMachineScope) createCOSClient() (*cos.Service, error) {
+	var cosInstanceName string
 	if m.IBMPowerVSCluster.Spec.CosInstance == nil || m.IBMPowerVSCluster.Spec.CosInstance.Name == "" {
-		return nil, fmt.Errorf("cannot create cos client cos instance name is not set")
+		cosInstanceName = fmt.Sprintf("%s-%s", m.IBMPowerVSCluster.GetName(), "cosinstance")
+	} else {
+		cosInstanceName = m.IBMPowerVSCluster.Spec.CosInstance.Name
 	}
-	cosInstanceName := m.IBMPowerVSCluster.Spec.CosInstance.Name
+
 	serviceInstance, err := m.ResourceClient.GetInstanceByName(cosInstanceName, resourcecontroller.CosResourceID, resourcecontroller.CosResourcePlanID)
 	if err != nil {
 		m.Error(err, "failed to get cos service instance", "name", cosInstanceName)
@@ -589,14 +596,9 @@ func (m *PowerVSMachineScope) createCOSClient() (*cos.Service, error) {
 		fmt.Printf("ibmcloud api key is not provided, set %s environmental variable", "IBMCLOUD_API_KEY")
 	}
 
-	region := m.IBMPowerVSCluster.Spec.CosInstance.BucketRegion
-	// if the bucket region is not set, use vpc region
+	region := m.bucketRegion()
 	if region == "" {
-		vpcDetails := m.IBMPowerVSCluster.Spec.VPC
-		if vpcDetails == nil || vpcDetails.Region == nil {
-			return nil, fmt.Errorf("failed to determine cos bucket region, both buckeet region and vpc region not set")
-		}
-		region = *vpcDetails.Region
+		return nil, fmt.Errorf("failed to determine cos bucket region, both bucket region and vpc region not set")
 	}
 
 	serviceEndpoint := fmt.Sprintf("s3.%s.%s", region, cosURLDomain)
@@ -1074,3 +1076,27 @@ func (m *PowerVSMachineScope) APIServerPort() int32 {
 	}
 	return infrav1beta2.DefaultAPIServerPort
 }
+
+// TODO: reuse getServiceName function instead.
+func (m *PowerVSMachineScope) bucketName() string {
+	if m.IBMPowerVSCluster.Spec.CosInstance != nil && m.IBMPowerVSCluster.Spec.CosInstance.BucketName != "" {
+		return m.IBMPowerVSCluster.Spec.CosInstance.BucketName
+	}
+	return fmt.Sprintf("%s-%s", m.IBMPowerVSCluster.GetName(), "cosbucket")
+}
+
+// TODO: duplicate function, optimize it.
+func (m *PowerVSMachineScope) bucketRegion() string {
+	var region string
+	if m.IBMPowerVSCluster.Spec.CosInstance != nil && m.IBMPowerVSCluster.Spec.CosInstance.BucketRegion != "" {
+		return m.IBMPowerVSCluster.Spec.CosInstance.BucketRegion
+	}
+	// if the bucket region is not set, use vpc region
+	if region == "" {
+		vpcDetails := m.IBMPowerVSCluster.Spec.VPC
+		if vpcDetails != nil && vpcDetails.Region != nil {
+			return *vpcDetails.Region
+		}
+	}
+	return region
+}

controllers/ibmpowervscluster_controller.go

@@ -195,12 +195,14 @@ func (r *IBMPowerVSClusterReconciler) reconcile(clusterScope *scope.PowerVSClust
 	}
 
 	// reconcile COSInstance
-	clusterScope.Info("Reconciling COSInstance")
-	if err := clusterScope.ReconcileCOSInstance(); err != nil {
-		conditions.MarkFalse(powerVSCluster, infrav1beta2.COSInstanceReadyCondition, infrav1beta2.COSInstanceReconciliationFailedReason, capiv1beta1.ConditionSeverityError, err.Error())
-		return reconcile.Result{}, err
+	if clusterScope.IBMPowerVSCluster.Spec.Ignition != nil {
+		clusterScope.Info("Reconciling COSInstance")
+		if err := clusterScope.ReconcileCOSInstance(); err != nil {
+			conditions.MarkFalse(powerVSCluster, infrav1beta2.COSInstanceReadyCondition, infrav1beta2.COSInstanceReconciliationFailedReason, capiv1beta1.ConditionSeverityError, err.Error())
+			return reconcile.Result{}, err
+		}
+		conditions.MarkTrue(powerVSCluster, infrav1beta2.COSInstanceReadyCondition)
 	}
-	conditions.MarkTrue(powerVSCluster, infrav1beta2.COSInstanceReadyCondition)
 
 	// update cluster object with loadbalancer host
 	loadBalancer := clusterScope.PublicLoadBalancer()
@@ -273,9 +275,11 @@ func (r *IBMPowerVSClusterReconciler) reconcileDelete(ctx context.Context, clust
 		allErrs = append(allErrs, errors.Wrapf(err, "failed to delete Power VS service instance"))
 	}
 
-	clusterScope.Info("Deleting COS service instance")
-	if err := clusterScope.DeleteCOSInstance(); err != nil {
-		allErrs = append(allErrs, errors.Wrapf(err, "failed to delete COS instance"))
+	if clusterScope.IBMPowerVSCluster.Spec.Ignition != nil {
+		clusterScope.Info("Deleting COS service instance")
+		if err := clusterScope.DeleteCOSInstance(); err != nil {
+			allErrs = append(allErrs, errors.Wrapf(err, "failed to delete COS instance"))
+		}
 	}
 
 	if len(allErrs) > 0 {