Skip to content

Fix issues with None platform #764

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

Merged
openshift-merge-robot merged 3 commits into from
Dec 14, 2021
Merged

Fix issues with None platform #764

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8876a9d
Adding None platform to hostedcluster allowed platforms
Dec 8, 2021
5b09eb8
Handle None platform nil infraCR
Dec 8, 2021
622af00
Add None platform infraCR
Dec 8, 2021
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
15 changes: 9 additions & 6 deletions hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -807,12 +807,15 @@ func (r *HostedClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reques
}

// Reconcile the CAPI Cluster resource
capiCluster := controlplaneoperator.CAPICluster(controlPlaneNamespace.Name, hcluster.Spec.InfraID)
_, err = createOrUpdate(ctx, r.Client, capiCluster, func() error {
return reconcileCAPICluster(capiCluster, hcluster, hcp, infraCR)
})
if err != nil {
return ctrl.Result{}, fmt.Errorf("failed to reconcile capi cluster: %w", err)
// In the None platform case, there is no CAPI provider/resources so infraCR is nil
if infraCR != nil {
capiCluster := controlplaneoperator.CAPICluster(controlPlaneNamespace.Name, hcluster.Spec.InfraID)
_, err = createOrUpdate(ctx, r.Client, capiCluster, func() error {
return reconcileCAPICluster(capiCluster, hcluster, hcp, infraCR)
})
if err != nil {
return ctrl.Result{}, fmt.Errorf("failed to reconcile capi cluster: %w", err)
}
}

// Reconcile the HostedControlPlane kubeconfig if one is reported
Expand Down
2 changes: 2 additions & 0 deletions hypershift-operator/controllers/hostedcluster/internal/platform/platform.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ func GetPlatform(hcluster *hyperv1.HostedCluster) (Platform, error) {
platform = aws.AWS{}
case hyperv1.IBMCloudPlatform:
platform = ibmcloud.IBMCloud{}
case hyperv1.NonePlatform:
platform = none.None{}
default:
return nil, fmt.Errorf("unsupported platform: %s", hcluster.Spec.Platform.Type)
}
Expand Down
2 changes: 2 additions & 0 deletions support/globalconfig/infrastructure.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,5 +57,7 @@ func ReconcileInfrastructure(infra *configv1.Infrastructure, hcp *hyperv1.Hosted
case hyperv1.IBMCloudPlatform:
infra.Status.PlatformStatus = &configv1.PlatformStatus{}
infra.Status.PlatformStatus.Type = configv1.IBMCloudPlatformType
case hyperv1.NonePlatform:
infra.Status.PlatformStatus = &configv1.PlatformStatus{}
Copy link
Member

@enxebre enxebre Dec 9, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Please just set
infra.Status.PlatformStatus.Type = configv1.PlatformType(hcp.Spec.Platform.Type)
for any case, no need to discriminate by platform for that. Then for case hyperv1.AWSPlatform: keep setting the infra.Status.PlatformStatus.AWS specifics.

Copy link
Contributor Author

@hardys hardys Dec 9, 2021

Choose a reason for hiding this comment

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

Ack thanks, I see that's already the default so I'll remove setting the Type

Copy link
Member

@enxebre enxebre Dec 10, 2021

Choose a reason for hiding this comment

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

we can drop ibm/none specifics and just do:

infra.Status.PlatformStatus = &configv1.PlatformStatus{}
infra.Status.Platform = configv1.PlatformType(hcp.Spec.Platform.Type)

switch hcp.Spec.Platform.Type {
	case hyperv1.AWSPlatform:
		infra.Spec.PlatformSpec.AWS = &configv1.AWSPlatformSpec{}
		infra.Status.PlatformStatus.Type = configv1.AWSPlatformType
		infra.Status.PlatformStatus.AWS = &configv1.AWSPlatformStatus{
			Region: hcp.Spec.Platform.AWS.Region,
		}
		tags := []configv1.AWSResourceTag{}
		for _, tag := range hcp.Spec.Platform.AWS.ResourceTags {
			tags = append(tags, configv1.AWSResourceTag{
				Key:   tag.Key,
				Value: tag.Value,
			})
		}
		infra.Status.PlatformStatus.AWS.ResourceTags = tags

I'll do that in a follow up PR.

Copy link
Member

@enxebre enxebre Dec 14, 2021

Choose a reason for hiding this comment

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

#782

}
}