From 5a0cc4a71c8ed3a448c3ed5c228d002f43f6d5e9 Mon Sep 17 00:00:00 2001 From: Ekaterina Kazakova Date: Wed, 4 Sep 2024 14:07:09 +0400 Subject: [PATCH] Verify requested providers readiness before admitting ManagedCluster creation Closes #236 --- internal/webhook/managedcluster_webhook.go | 92 +++++--- .../webhook/managedcluster_webhook_test.go | 222 ++++++++++++++---- 2 files changed, 234 insertions(+), 80 deletions(-) diff --git a/internal/webhook/managedcluster_webhook.go b/internal/webhook/managedcluster_webhook.go index bb2b8a26..857a9a92 100644 --- a/internal/webhook/managedcluster_webhook.go +++ b/internal/webhook/managedcluster_webhook.go @@ -61,14 +61,18 @@ func (v *ManagedClusterValidator) ValidateCreate(ctx context.Context, obj runtim if !ok { return nil, apierrors.NewBadRequest(fmt.Sprintf("expected ManagedCluster but got a %T", obj)) } - template, err := v.getManagedClusterTemplate(ctx, managedCluster.Spec.Template) + template, err := v.getTemplate(ctx, managedCluster.Spec.Template) if err != nil { return nil, fmt.Errorf("%s: %v", InvalidManagedClusterErr, err) } - err = v.isTemplateValid(ctx, template) + err = v.isTemplateValid(template) if err != nil { return nil, fmt.Errorf("%s: %v", InvalidManagedClusterErr, err) } + err = v.checkComponentsHealth(ctx, template) + if err != nil { + return nil, fmt.Errorf("%s: components verification failed: %v", InvalidManagedClusterErr, err) + } return nil, nil } @@ -78,11 +82,11 @@ func (v *ManagedClusterValidator) ValidateUpdate(ctx context.Context, _ runtime. if !ok { return nil, apierrors.NewBadRequest(fmt.Sprintf("expected ManagedCluster but got a %T", newObj)) } - template, err := v.getManagedClusterTemplate(ctx, newManagedCluster.Spec.Template) + template, err := v.getTemplate(ctx, newManagedCluster.Spec.Template) if err != nil { return nil, fmt.Errorf("%s: %v", InvalidManagedClusterErr, err) } - err = v.isTemplateValid(ctx, template) + err = v.isTemplateValid(template) if err != nil { return nil, fmt.Errorf("%s: %v", InvalidManagedClusterErr, err) } @@ -105,11 +109,11 @@ func (v *ManagedClusterValidator) Default(ctx context.Context, obj runtime.Objec if managedCluster.Spec.Config != nil { return nil } - template, err := v.getManagedClusterTemplate(ctx, managedCluster.Spec.Template) + template, err := v.getTemplate(ctx, managedCluster.Spec.Template) if err != nil { return fmt.Errorf("could not get template for the managedcluster: %s", err) } - err = v.isTemplateValid(ctx, template) + err = v.isTemplateValid(template) if err != nil { return fmt.Errorf("template is invalid: %s", err) } @@ -121,7 +125,7 @@ func (v *ManagedClusterValidator) Default(ctx context.Context, obj runtime.Objec return nil } -func (v *ManagedClusterValidator) getManagedClusterTemplate(ctx context.Context, templateName string) (*v1alpha1.Template, error) { +func (v *ManagedClusterValidator) getTemplate(ctx context.Context, templateName string) (*v1alpha1.Template, error) { template := &v1alpha1.Template{} templateRef := types.NamespacedName{Name: templateName, Namespace: v.SystemNamespace} if err := v.Get(ctx, templateRef, template); err != nil { @@ -130,22 +134,18 @@ func (v *ManagedClusterValidator) getManagedClusterTemplate(ctx context.Context, return template, nil } -func (v *ManagedClusterValidator) isTemplateValid(ctx context.Context, template *v1alpha1.Template) error { +func (v *ManagedClusterValidator) isTemplateValid(template *v1alpha1.Template) error { if template.Status.Type != v1alpha1.TemplateTypeDeployment { return fmt.Errorf("the template should be of the deployment type. Current: %s", template.Status.Type) } if !template.Status.Valid { return fmt.Errorf("the template is not valid: %s", template.Status.ValidationError) } - err := v.verifyProviders(ctx, template) - if err != nil { - return fmt.Errorf("providers verification failed: %v", err) - } return nil } -func (v *ManagedClusterValidator) verifyProviders(ctx context.Context, template *v1alpha1.Template) error { - requiredProviders := template.Status.Providers +func (v *ManagedClusterValidator) checkComponentsHealth(ctx context.Context, managedClusterTemplate *v1alpha1.Template) error { + requiredProviders := managedClusterTemplate.Status.Providers management := &v1alpha1.Management{} managementRef := types.NamespacedName{Name: v1alpha1.ManagementName} if err := v.Get(ctx, managementRef, management); err != nil { @@ -153,34 +153,68 @@ func (v *ManagedClusterValidator) verifyProviders(ctx context.Context, template } exposedProviders := management.Status.AvailableProviders - missingProviders := make(map[string][]string) - missingProviders["bootstrap"] = getMissingProviders(exposedProviders.BootstrapProviders, requiredProviders.BootstrapProviders) - missingProviders["control plane"] = getMissingProviders(exposedProviders.ControlPlaneProviders, requiredProviders.ControlPlaneProviders) - missingProviders["infrastructure"] = getMissingProviders(exposedProviders.InfrastructureProviders, requiredProviders.InfrastructureProviders) + missingComponents := make(map[string][]string) + + var failedComponents []string + componentsErrors := make(map[string]string) + for component, status := range management.Status.Components { + if !status.Success { + template, err := v.getTemplate(ctx, component) + if err != nil { + return err + } + if template.Status.Type == v1alpha1.TemplateTypeCore { + missingComponents["core components"] = append(missingComponents["core components"], component) + failedComponents = append(failedComponents, component) + componentsErrors[component] = status.Error + } + if template.Status.Type == v1alpha1.TemplateTypeProvider { + if oneOrMoreProviderFailed(template.Status.Providers.BootstrapProviders, requiredProviders.BootstrapProviders) || + oneOrMoreProviderFailed(template.Status.Providers.ControlPlaneProviders, requiredProviders.ControlPlaneProviders) || + oneOrMoreProviderFailed(template.Status.Providers.InfrastructureProviders, requiredProviders.InfrastructureProviders) { + failedComponents = append(failedComponents, component) + componentsErrors[component] = status.Error + } + } + } + } - var errs []error - for providerType, missing := range missingProviders { + missingComponents["bootstrap providers"] = getMissingProviders(exposedProviders.BootstrapProviders, requiredProviders.BootstrapProviders) + missingComponents["control plane providers"] = getMissingProviders(exposedProviders.ControlPlaneProviders, requiredProviders.ControlPlaneProviders) + missingComponents["infrastructure providers"] = getMissingProviders(exposedProviders.InfrastructureProviders, requiredProviders.InfrastructureProviders) + + errs := make([]error, 0, len(missingComponents)+len(failedComponents)) + for componentType, missing := range missingComponents { if len(missing) > 0 { - sort.Slice(missing, func(i, j int) bool { - return missing[i] < missing[j] - }) - errs = append(errs, fmt.Errorf("one or more required %s providers are not deployed yet: %v", providerType, missing)) + sort.Strings(missing) + errs = append(errs, fmt.Errorf("one or more required %s are not deployed yet: %v", componentType, missing)) } } + sort.Slice(errs, func(i, j int) bool { + return errs[i].Error() < errs[j].Error() + }) + + sort.Strings(failedComponents) + for _, failedComponent := range failedComponents { + errs = append(errs, fmt.Errorf("%s installation failed: %s", failedComponent, componentsErrors[failedComponent])) + } if len(errs) > 0 { - sort.Slice(errs, func(i, j int) bool { - return errs[i].Error() < errs[j].Error() - }) return errors.Join(errs...) } return nil } func getMissingProviders(exposedProviders []string, requiredProviders []string) []string { - exposedBootstrapProviders := utils.SliceToMapKeys[[]string, map[string]struct{}](exposedProviders) - diff, isSubset := utils.DiffSliceSubset[[]string, map[string]struct{}](requiredProviders, exposedBootstrapProviders) + exposedProvidersMap := utils.SliceToMapKeys[[]string, map[string]struct{}](exposedProviders) + diff, isSubset := utils.DiffSliceSubset[[]string, map[string]struct{}](requiredProviders, exposedProvidersMap) if !isSubset { return diff } return []string{} } + +func oneOrMoreProviderFailed(failedProviders []string, requiredProviders []string) bool { + failedProvidersMap := utils.SliceToMapKeys[[]string, map[string]struct{}](failedProviders) + _, isSubset := utils.DiffSliceSubset[[]string, map[string]struct{}](requiredProviders, failedProvidersMap) + return isSubset +} diff --git a/internal/webhook/managedcluster_webhook_test.go b/internal/webhook/managedcluster_webhook_test.go index c17dadaf..bc41cf61 100644 --- a/internal/webhook/managedcluster_webhook_test.go +++ b/internal/webhook/managedcluster_webhook_test.go @@ -20,6 +20,7 @@ import ( "testing" . "github.com/onsi/gomega" + admissionv1 "k8s.io/api/admissionregistration/v1" "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" @@ -34,6 +35,8 @@ import ( var ( testTemplateName = "template-test" + newTemplateName = "template-test-2" + capzTemplateName = "cluster-api-provider-azure" mgmt = management.NewManagement( management.WithAvailableProviders(v1alpha1.Providers{ @@ -41,23 +44,38 @@ var ( BootstrapProviders: []string{"k0s"}, ControlPlaneProviders: []string{"k0s"}, }), + management.WithComponentsStatus(map[string]v1alpha1.ComponentStatus{ + v1alpha1.DefaultCoreHMCTemplate: {Success: true}, + v1alpha1.DefaultCoreCAPITemplate: {Success: true}, + }), ) +) - createAndUpdateTests = []struct { - name string - managedCluster *v1alpha1.ManagedCluster - existingObjects []runtime.Object - err string - warnings admission.Warnings +func TestDeploymentValidateCreateOrUpdate(t *testing.T) { + g := NewWithT(t) + + ctx := context.Background() + + tests := []struct { + name string + operation admissionv1.OperationType + oldManagedCluster *v1alpha1.ManagedCluster + newManagedCluster *v1alpha1.ManagedCluster + managedCluster *v1alpha1.ManagedCluster + existingObjects []runtime.Object + err string + warnings admission.Warnings }{ { - name: "should fail if the template is unset", - managedCluster: managedcluster.NewManagedCluster(), - err: "the ManagedCluster is invalid: templates.hmc.mirantis.com \"\" not found", + name: "create - should fail if the template is unset", + operation: admissionv1.Create, + newManagedCluster: managedcluster.NewManagedCluster(), + err: "the ManagedCluster is invalid: templates.hmc.mirantis.com \"\" not found", }, { - name: "should fail if the template is not found in hmc-system namespace", - managedCluster: managedcluster.NewManagedCluster(managedcluster.WithTemplate(testTemplateName)), + name: "create - should fail if the template is not found in hmc-system namespace", + operation: admissionv1.Create, + newManagedCluster: managedcluster.NewManagedCluster(managedcluster.WithTemplate(testTemplateName)), existingObjects: []runtime.Object{ mgmt, template.NewTemplate( @@ -68,8 +86,9 @@ var ( err: fmt.Sprintf("the ManagedCluster is invalid: templates.hmc.mirantis.com \"%s\" not found", testTemplateName), }, { - name: "should fail if the template was found but is invalid (type is unset)", - managedCluster: managedcluster.NewManagedCluster(managedcluster.WithTemplate(testTemplateName)), + name: "create - should fail if the template was found but is invalid (type is unset)", + operation: admissionv1.Create, + newManagedCluster: managedcluster.NewManagedCluster(managedcluster.WithTemplate(testTemplateName)), existingObjects: []runtime.Object{ mgmt, template.NewTemplate(template.WithName(testTemplateName)), @@ -77,8 +96,9 @@ var ( err: "the ManagedCluster is invalid: the template should be of the deployment type. Current: ", }, { - name: "should fail if the template was found but is invalid (some validation error)", - managedCluster: managedcluster.NewManagedCluster(managedcluster.WithTemplate(testTemplateName)), + name: "create - should fail if the template was found but is invalid (some validation error)", + operation: admissionv1.Create, + newManagedCluster: managedcluster.NewManagedCluster(managedcluster.WithTemplate(testTemplateName)), existingObjects: []runtime.Object{ mgmt, template.NewTemplate( @@ -93,14 +113,57 @@ var ( err: "the ManagedCluster is invalid: the template is not valid: validation error example", }, { - name: "should fail if one or more requested providers are not available yet", - managedCluster: managedcluster.NewManagedCluster(managedcluster.WithTemplate(testTemplateName)), + name: "create - should fail if one or more requested providers are not available yet", + operation: admissionv1.Create, + newManagedCluster: managedcluster.NewManagedCluster(managedcluster.WithTemplate(testTemplateName)), + existingObjects: []runtime.Object{ + management.NewManagement( + management.WithAvailableProviders(v1alpha1.Providers{ + InfrastructureProviders: []string{"aws"}, + BootstrapProviders: []string{"k0s"}, + }), + management.WithComponentsStatus(map[string]v1alpha1.ComponentStatus{ + v1alpha1.DefaultCoreHMCTemplate: {Success: true}, + v1alpha1.DefaultCoreCAPITemplate: {Success: true}, + }), + ), + template.NewTemplate( + template.WithName(testTemplateName), + template.WithTypeStatus(v1alpha1.TemplateTypeDeployment), + template.WithProvidersStatus(v1alpha1.Providers{ + InfrastructureProviders: []string{"azure"}, + BootstrapProviders: []string{"k0s"}, + ControlPlaneProviders: []string{"k0s"}, + }), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), + ), + }, + err: `the ManagedCluster is invalid: components verification failed: one or more required control plane providers are not deployed yet: [k0s] +one or more required infrastructure providers are not deployed yet: [azure]`, + }, + { + name: "create - should fail if one or more providers and core components are not ready yet", + operation: admissionv1.Create, + newManagedCluster: managedcluster.NewManagedCluster(managedcluster.WithTemplate(testTemplateName)), existingObjects: []runtime.Object{ management.NewManagement( management.WithAvailableProviders(v1alpha1.Providers{ InfrastructureProviders: []string{"aws"}, BootstrapProviders: []string{"k0s"}, }), + management.WithComponentsStatus(map[string]v1alpha1.ComponentStatus{ + v1alpha1.DefaultCoreHMCTemplate: {Success: true}, + v1alpha1.DefaultCoreCAPITemplate: { + Success: false, + Error: "cluster-api template is invalid", + }, + }), + ), + template.NewTemplate(template.WithName(v1alpha1.DefaultCoreHMCTemplate)), + template.NewTemplate( + template.WithName(v1alpha1.DefaultCoreCAPITemplate), + template.WithTypeStatus(v1alpha1.TemplateTypeCore), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: false}), ), template.NewTemplate( template.WithName(testTemplateName), @@ -113,11 +176,51 @@ var ( template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), ), }, - err: "the ManagedCluster is invalid: providers verification failed: one or more required control plane providers are not deployed yet: [k0s]\none or more required infrastructure providers are not deployed yet: [azure]", + err: `the ManagedCluster is invalid: components verification failed: one or more required control plane providers are not deployed yet: [k0s] +one or more required core components are not deployed yet: [cluster-api] +one or more required infrastructure providers are not deployed yet: [azure] +cluster-api installation failed: cluster-api template is invalid`, }, { - name: "should succeed", - managedCluster: managedcluster.NewManagedCluster(managedcluster.WithTemplate(testTemplateName)), + name: "create - should succeed if one or more providers failed but it is not required", + operation: admissionv1.Create, + newManagedCluster: managedcluster.NewManagedCluster(managedcluster.WithTemplate(testTemplateName)), + existingObjects: []runtime.Object{ + management.NewManagement( + management.WithAvailableProviders(v1alpha1.Providers{ + InfrastructureProviders: []string{"aws", "azure"}, + BootstrapProviders: []string{"k0s"}, + ControlPlaneProviders: []string{"k0s"}, + }), + management.WithComponentsStatus(map[string]v1alpha1.ComponentStatus{ + v1alpha1.DefaultCoreHMCTemplate: {Success: true}, + v1alpha1.DefaultCoreCAPITemplate: {Success: true}, + capzTemplateName: { + Success: false, + Error: "cluster API provider Azure deployment failed", + }, + }), + ), + template.NewTemplate( + template.WithName(capzTemplateName), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: false}), + ), + template.NewTemplate( + template.WithName(testTemplateName), + template.WithTypeStatus(v1alpha1.TemplateTypeDeployment), + template.WithProvidersStatus(v1alpha1.Providers{ + InfrastructureProviders: []string{"aws"}, + BootstrapProviders: []string{"k0s"}, + ControlPlaneProviders: []string{"k0s"}, + }), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), + ), + }, + }, + { + name: "create - should succeed", + operation: admissionv1.Create, + newManagedCluster: managedcluster.NewManagedCluster(managedcluster.WithTemplate(testTemplateName)), existingObjects: []runtime.Object{ mgmt, template.NewTemplate( @@ -132,44 +235,61 @@ var ( ), }, }, - } -) -func TestManagedClusterValidateCreate(t *testing.T) { - g := NewWithT(t) + { + name: "update - should fail if the template is unset", + operation: admissionv1.Update, + oldManagedCluster: managedcluster.NewManagedCluster(managedcluster.WithTemplate(testTemplateName)), + newManagedCluster: managedcluster.NewManagedCluster(), + err: "the ManagedCluster is invalid: templates.hmc.mirantis.com \"\" not found", + }, + { + name: "update - should fail if the new template is not found in hmc-system namespace", + operation: admissionv1.Update, + oldManagedCluster: managedcluster.NewManagedCluster(managedcluster.WithTemplate(testTemplateName)), + newManagedCluster: managedcluster.NewManagedCluster(managedcluster.WithTemplate(newTemplateName)), + existingObjects: []runtime.Object{ + mgmt, + template.NewTemplate( + template.WithName(newTemplateName), + template.WithNamespace("default"), + ), + }, + err: fmt.Sprintf("the ManagedCluster is invalid: templates.hmc.mirantis.com \"%s\" not found", newTemplateName), + }, + { + name: "update - should fail if the new template was found but is invalid (some validation error)", + operation: admissionv1.Update, + oldManagedCluster: managedcluster.NewManagedCluster(managedcluster.WithTemplate(testTemplateName)), + newManagedCluster: managedcluster.NewManagedCluster(managedcluster.WithTemplate(newTemplateName)), + existingObjects: []runtime.Object{ + mgmt, + template.NewTemplate( + template.WithName(newTemplateName), + template.WithTypeStatus(v1alpha1.TemplateTypeDeployment), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{ + Valid: false, + ValidationError: "validation error example", + }), + ), + }, + err: "the ManagedCluster is invalid: the template is not valid: validation error example", + }, + } - ctx := context.Background() - for _, tt := range createAndUpdateTests { + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { c := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(tt.existingObjects...).Build() validator := &ManagedClusterValidator{Client: c, SystemNamespace: utils.DefaultSystemNamespace} - warn, err := validator.ValidateCreate(ctx, tt.managedCluster) - if tt.err != "" { - g.Expect(err).To(HaveOccurred()) - if err.Error() != tt.err { - t.Fatalf("expected error '%s', got error: %s", tt.err, err.Error()) - } - } else { - g.Expect(err).To(Succeed()) - } - if len(tt.warnings) > 0 { - g.Expect(warn).To(Equal(tt.warnings)) + var warn []string + var err error + if tt.operation == admissionv1.Create { + warn, err = validator.ValidateCreate(ctx, tt.newManagedCluster) + } else if tt.operation == admissionv1.Update { + warn, err = validator.ValidateUpdate(ctx, tt.oldManagedCluster, tt.newManagedCluster) } else { - g.Expect(warn).To(BeEmpty()) + t.Fatalf("Invalid test case, expecting operation [%s,%s], got %s", admissionv1.Create, admissionv1.Update, tt.operation) } - }) - } -} - -func TestManagedClusterValidateUpdate(t *testing.T) { - g := NewWithT(t) - - ctx := context.Background() - for _, tt := range createAndUpdateTests { - t.Run(tt.name, func(t *testing.T) { - c := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(tt.existingObjects...).Build() - validator := &ManagedClusterValidator{Client: c, SystemNamespace: utils.DefaultSystemNamespace} - warn, err := validator.ValidateUpdate(ctx, managedcluster.NewManagedCluster(), tt.managedCluster) if tt.err != "" { g.Expect(err).To(HaveOccurred()) if err.Error() != tt.err {