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.

Sign up for GitHub

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

Jump to bottom

Fix cos flow #1664

Merged
merged 1 commit into from
Mar 26, 2024
Merged

Fix cos flow #1664

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 0 additions & 4 deletions api/v1beta2/ibmpowervscluster_webhook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,6 @@ func (r *IBMPowerVSCluster) validateIBMPowerVSClusterCreateInfraPrereq() *field.
return field.Invalid(field.NewPath("spec.resourceGroup"), r.Spec.ResourceGroup, "value of resource group is empty")
}

if r.Spec.Ignition == nil {
return nil
}
Comment on lines -136 to -138
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, You are supposed remove it from here, Not the whole file, We still need webhook for verifying zone vpc region.


// 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 {
Expand Down
2 changes: 2 additions & 0 deletions api/v1beta2/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
)
Expand Down
40 changes: 25 additions & 15 deletions cloud/scope/powervs_cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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":
Expand All @@ -1522,7 +1514,7 @@ func (s *PowerVSClusterScope) checkCOSBucket() (bool, error) {

func (s *PowerVSClusterScope) createCOSBucket() error {
input := &s3.CreateBucketInput{
Bucket: ptr.To(s.COSInstance().BucketName),
Bucket: ptr.To(*s.GetServiceName(infrav1beta2.ResourceTypeCOSBucket)),
}
_, err := s.COSClient.CreateBucket(input)
if err == nil {
Expand All @@ -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) {
Expand Down Expand Up @@ -1693,6 +1685,11 @@ func (s *PowerVSClusterScope) GetServiceName(resourceType infrav1beta2.ResourceT
return ptr.To(fmt.Sprintf("%s-cosinstance", s.InfraCluster()))
}
return &s.COSInstance().Name
case infrav1beta2.ResourceTypeCOSBucket:
if s.COSInstance() == nil || s.COSInstance().BucketName == "" {
return ptr.To(fmt.Sprintf("%s-cosbucket", s.InfraCluster()))
}
return &s.COSInstance().BucketName
case infrav1beta2.ResourceTypeSubnet:
return ptr.To(fmt.Sprintf("%s-vpcsubnet", s.InfraCluster()))
case infrav1beta2.ResourceTypeLoadBalancer:
Expand Down Expand Up @@ -1977,3 +1974,16 @@ func (s *PowerVSClusterScope) isResourceCreatedByController(resourceType infrav1
}
return false
}

// TODO: duplicate function, optimize it.
func (s *PowerVSClusterScope) bucketRegion() string {
if s.COSInstance() != nil && s.COSInstance().BucketRegion != "" {
return s.COSInstance().BucketRegion
}
// if the bucket region is not set, use vpc region
vpcDetails := s.VPC()
if vpcDetails != nil && vpcDetails.Region != nil {
return *vpcDetails.Region
}
return ""
}
49 changes: 36 additions & 13 deletions cloud/scope/powervs_machine.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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,
Expand Down Expand Up @@ -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),
})
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -1074,3 +1076,24 @@ func (m *PowerVSMachineScope) APIServerPort() int32 {
}
return infrav1beta2.DefaultAPIServerPort
}

// TODO: reuse getServiceName function instead.
func (m *PowerVSMachineScope) bucketName() string {
Prajyot-Parab marked this conversation as resolved.
Show resolved Hide resolved
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 {
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
vpcDetails := m.IBMPowerVSCluster.Spec.VPC
if vpcDetails != nil && vpcDetails.Region != nil {
return *vpcDetails.Region
}
return ""
}
20 changes: 12 additions & 8 deletions controllers/ibmpowervscluster_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,12 +196,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()
Expand Down Expand Up @@ -274,9 +276,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 {
Expand Down