Skip to content

Commit

Permalink
Validate mgmt providers removal
Browse files Browse the repository at this point in the history
* forbid Management .spec.providers items
  removal if a ManagedCluster utilizes one
  of the removed providers
* amends to CRDs descriptions

Closes #554
  • Loading branch information
zerospiel committed Nov 1, 2024
1 parent 27d6593 commit 0668095
Show file tree
Hide file tree
Showing 16 changed files with 276 additions and 127 deletions.
2 changes: 1 addition & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ issues:
- text: "Unhandled error in call to function fmt.Print*"
linters:
- revive
- path: cmd/main.go
- path: (cmd/main\.go)|(_test\.go)
linters:
- maintidx
- path: test/
Expand Down
6 changes: 2 additions & 4 deletions api/v1alpha1/clustertemplate_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,8 @@ type ClusterTemplateSpec struct {
ProviderContracts CompatibilityContracts `json:"providerContracts,omitempty"`
// Kubernetes exact version in the SemVer format provided by this ClusterTemplate.
KubernetesVersion string `json:"k8sVersion,omitempty"`
// Providers represent required CAPI providers with supported contract versions.
// Providers represent required CAPI providers.
// Should be set if not present in the Helm chart metadata.
// Compatibility attributes are optional to be defined.
Providers Providers `json:"providers,omitempty"`
}

Expand All @@ -57,8 +56,7 @@ type ClusterTemplateStatus struct {
ProviderContracts CompatibilityContracts `json:"providerContracts,omitempty"`
// Kubernetes exact version in the SemVer format provided by this ClusterTemplate.
KubernetesVersion string `json:"k8sVersion,omitempty"`
// Providers represent required CAPI providers with supported contract versions
// if the latter has been given.
// Providers represent required CAPI providers.
Providers Providers `json:"providers,omitempty"`

TemplateStatusCommon `json:",inline"`
Expand Down
3 changes: 1 addition & 2 deletions api/v1alpha1/management_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,7 @@ type ManagementStatus struct {
Components map[string]ComponentStatus `json:"components,omitempty"`
// Release indicates the current Release object.
Release string `json:"release,omitempty"`
// AvailableProviders holds all CAPI providers available along with
// their supported contract versions, if specified in ProviderTemplates, on the Management cluster.
// AvailableProviders holds all available CAPI providers.
AvailableProviders Providers `json:"availableProviders,omitempty"`
// ObservedGeneration is the last observed generation.
ObservedGeneration int64 `json:"observedGeneration,omitempty"`
Expand Down
6 changes: 2 additions & 4 deletions api/v1alpha1/providertemplate_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,15 @@ const ProviderTemplateKind = "ProviderTemplate"
type ProviderTemplateSpec struct {
Helm HelmSpec `json:"helm,omitempty"`
CAPIContracts CompatibilityContracts `json:"capiContracts,omitempty"`
// Providers represent exposed CAPI providers with supported contract versions.
// Providers represent exposed CAPI providers.
// Should be set if not present in the Helm chart metadata.
// Supported contract versions are optional to be defined.
Providers Providers `json:"providers,omitempty"`
}

// ProviderTemplateStatus defines the observed state of ProviderTemplate
type ProviderTemplateStatus struct {
CAPIContracts CompatibilityContracts `json:"capiContracts,omitempty"`
// Providers represent exposed CAPI providers with supported contract versions
// if the latter has been given.
// Providers represent exposed CAPI providers.
Providers Providers `json:"providers,omitempty"`

TemplateStatusCommon `json:",inline"`
Expand Down
29 changes: 8 additions & 21 deletions internal/controller/managedcluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/event"
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/predicate"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

hmc "github.com/Mirantis/hmc/api/v1alpha1"
"github.com/Mirantis/hmc/internal/credspropagation"
Expand Down Expand Up @@ -95,18 +94,15 @@ func (r *ManagedClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reque
l.Error(err, "Failed to get Management object")
return ctrl.Result{}, err
}
if err := telemetry.TrackManagedClusterCreate(
string(mgmt.UID), string(managedCluster.UID), managedCluster.Spec.Template, managedCluster.Spec.DryRun); err != nil {
if err := telemetry.TrackManagedClusterCreate(string(mgmt.UID), string(managedCluster.UID), managedCluster.Spec.Template, managedCluster.Spec.DryRun); err != nil {
l.Error(err, "Failed to track ManagedCluster creation")
}
}

return r.Update(ctx, managedCluster)
}

func (r *ManagedClusterReconciler) setStatusFromClusterStatus(
ctx context.Context, managedCluster *hmc.ManagedCluster,
) (bool, error) {
func (r *ManagedClusterReconciler) setStatusFromClusterStatus(ctx context.Context, managedCluster *hmc.ManagedCluster) (requeue bool, _ error) {
l := ctrl.LoggerFrom(ctx)

resourceConditions, err := status.GetResourceConditions(ctx, managedCluster.Namespace, r.DynamicClient, schema.GroupVersionResource{
Expand All @@ -115,8 +111,7 @@ func (r *ManagedClusterReconciler) setStatusFromClusterStatus(
Resource: "clusters",
}, labels.SelectorFromSet(map[string]string{hmc.FluxHelmChartNameKey: managedCluster.Name}).String())
if err != nil {
notFoundErr := status.ResourceNotFoundError{}
if errors.As(err, &notFoundErr) {
if errors.As(err, &status.ResourceNotFoundError{}) {
l.Info(err.Error())
return true, nil
}
Expand All @@ -130,7 +125,7 @@ func (r *ManagedClusterReconciler) setStatusFromClusterStatus(
}

if metaCondition.Reason == "" && metaCondition.Status == "True" {
metaCondition.Reason = "Succeeded"
metaCondition.Reason = hmc.SucceededReason
}
apimeta.SetStatusCondition(managedCluster.GetConditions(), metaCondition)
}
Expand Down Expand Up @@ -756,20 +751,12 @@ func (r *ManagedClusterReconciler) SetupWithManager(mgr ctrl.Manager) error {
For(&hmc.ManagedCluster{}).
Watches(&hcv2.HelmRelease{},
handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, o client.Object) []ctrl.Request {
managedCluster := hmc.ManagedCluster{}
managedClusterRef := client.ObjectKey{
Namespace: o.GetNamespace(),
Name: o.GetName(),
}
err := r.Client.Get(ctx, managedClusterRef, &managedCluster)
if err != nil {
managedClusterRef := client.ObjectKeyFromObject(o)
if err := r.Client.Get(ctx, managedClusterRef, &hmc.ManagedCluster{}); err != nil {
return []ctrl.Request{}
}
return []reconcile.Request{
{
NamespacedName: managedClusterRef,
},
}

return []ctrl.Request{{NamespacedName: managedClusterRef}}
}),
).
Watches(&hmc.ClusterTemplateChain{},
Expand Down
13 changes: 4 additions & 9 deletions internal/controller/management_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ type ManagementReconciler struct {
client.Client
Scheme *runtime.Scheme
Config *rest.Config
SystemNamespace string
DynamicClient *dynamic.DynamicClient
SystemNamespace string
CreateTemplateManagement bool
}

Expand Down Expand Up @@ -246,8 +246,7 @@ func (r *ManagementReconciler) checkProviderStatus(ctx context.Context, provider
labels.SelectorFromSet(map[string]string{hmc.FluxHelmChartNameKey: providerTemplateName}).String(),
)
if err != nil {
notFoundErr := status.ResourceNotFoundError{}
if errors.As(err, &notFoundErr) {
if errors.As(err, &status.ResourceNotFoundError{}) {
// Check the next resource type.
continue
}
Expand All @@ -263,16 +262,12 @@ func (r *ManagementReconciler) checkProviderStatus(ctx context.Context, provider
}

if len(falseConditionTypes) > 0 {
errs = errors.Join(fmt.Errorf("%s: %s is not yet ready, has false conditions: %s",
errs = errors.Join(errs, fmt.Errorf("%s: %s is not yet ready, has false conditions: %s",
resourceConditions.Name, resourceConditions.Kind, strings.Join(falseConditionTypes, ", ")))
}
}

if errs != nil {
return errs
}

return nil
return errs
}

func (r *ManagementReconciler) Delete(ctx context.Context, management *hmc.Management) (ctrl.Result, error) {
Expand Down
3 changes: 1 addition & 2 deletions internal/controller/template_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -342,8 +342,7 @@ func (r *ClusterTemplateReconciler) validateCompatibilityAttrs(ctx context.Conte
exposedProviders, requiredProviders := management.Status.AvailableProviders, template.Status.Providers

l := ctrl.LoggerFrom(ctx)
l.V(1).Info("providers to check", "exposed", exposedProviders, "required", requiredProviders,
"exposed_capi_contract_versions", management.Status.CAPIContracts, "required_provider_contract_versions", template.Status.ProviderContracts)
l.V(1).Info("providers to check", "exposed", exposedProviders, "required", requiredProviders)

var (
merr error
Expand Down
36 changes: 18 additions & 18 deletions internal/webhook/managedcluster_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,11 @@ func TestManagedClusterValidateCreate(t *testing.T) {
cred,
template.NewClusterTemplate(
template.WithName(testTemplateName),
template.WithProvidersStatus(v1alpha1.Providers{
template.WithProvidersStatus(
"infrastructure-aws",
"control-plane-k0smotron",
"bootstrap-k0smotron",
}),
),
template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}),
),
},
Expand Down Expand Up @@ -170,11 +170,11 @@ func TestManagedClusterValidateCreate(t *testing.T) {
mgmt,
template.NewClusterTemplate(
template.WithName(testTemplateName),
template.WithProvidersStatus(v1alpha1.Providers{
template.WithProvidersStatus(
"infrastructure-aws",
"control-plane-k0smotron",
"bootstrap-k0smotron",
}),
),
template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}),
),
},
Expand All @@ -199,11 +199,11 @@ func TestManagedClusterValidateCreate(t *testing.T) {
),
template.NewClusterTemplate(
template.WithName(testTemplateName),
template.WithProvidersStatus(v1alpha1.Providers{
template.WithProvidersStatus(
"infrastructure-aws",
"control-plane-k0smotron",
"bootstrap-k0smotron",
}),
),
template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}),
),
},
Expand All @@ -226,11 +226,11 @@ func TestManagedClusterValidateCreate(t *testing.T) {
),
template.NewClusterTemplate(
template.WithName(testTemplateName),
template.WithProvidersStatus(v1alpha1.Providers{
template.WithProvidersStatus(
"infrastructure-azure",
"control-plane-k0smotron",
"bootstrap-k0smotron",
}),
),
template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}),
),
},
Expand Down Expand Up @@ -313,20 +313,20 @@ func TestManagedClusterValidateUpdate(t *testing.T) {
template.NewClusterTemplate(
template.WithName(testTemplateName),
template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}),
template.WithProvidersStatus(v1alpha1.Providers{
template.WithProvidersStatus(
"infrastructure-aws",
"control-plane-k0smotron",
"bootstrap-k0smotron",
}),
),
),
template.NewClusterTemplate(
template.WithName(upgradeTargetTemplateName),
template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}),
template.WithProvidersStatus(v1alpha1.Providers{
template.WithProvidersStatus(
"infrastructure-aws",
"control-plane-k0smotron",
"bootstrap-k0smotron",
}),
),
),
},
warnings: admission.Warnings{fmt.Sprintf("Cluster can't be upgraded from %s to %s. This upgrade sequence is not allowed", testTemplateName, upgradeTargetTemplateName)},
Expand All @@ -348,20 +348,20 @@ func TestManagedClusterValidateUpdate(t *testing.T) {
template.NewClusterTemplate(
template.WithName(testTemplateName),
template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}),
template.WithProvidersStatus(v1alpha1.Providers{
template.WithProvidersStatus(
"infrastructure-aws",
"control-plane-k0smotron",
"bootstrap-k0smotron",
}),
),
),
template.NewClusterTemplate(
template.WithName(newTemplateName),
template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}),
template.WithProvidersStatus(v1alpha1.Providers{
template.WithProvidersStatus(
"infrastructure-aws",
"control-plane-k0smotron",
"bootstrap-k0smotron",
}),
),
),
},
},
Expand All @@ -386,11 +386,11 @@ func TestManagedClusterValidateUpdate(t *testing.T) {
Valid: false,
ValidationError: "validation error example",
}),
template.WithProvidersStatus(v1alpha1.Providers{
template.WithProvidersStatus(
"infrastructure-aws",
"control-plane-k0smotron",
"bootstrap-k0smotron",
}),
),
),
},
},
Expand Down
Loading

0 comments on commit 0668095

Please sign in to comment.