From 7d8af9c3931d1e12e256f729f401477d72e9eba0 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 Deployment creation Closes #236 --- internal/webhook/deployment_webhook.go | 92 ++++++--- internal/webhook/deployment_webhook_test.go | 213 +++++++++++++++----- 2 files changed, 229 insertions(+), 76 deletions(-) diff --git a/internal/webhook/deployment_webhook.go b/internal/webhook/deployment_webhook.go index c4efa7ad..0aad57f6 100644 --- a/internal/webhook/deployment_webhook.go +++ b/internal/webhook/deployment_webhook.go @@ -61,14 +61,18 @@ func (v *DeploymentValidator) ValidateCreate(ctx context.Context, obj runtime.Ob if !ok { return nil, apierrors.NewBadRequest(fmt.Sprintf("expected Deployment but got a %T", obj)) } - template, err := v.getDeploymentTemplate(ctx, deployment.Spec.Template) + template, err := v.getTemplate(ctx, deployment.Spec.Template) if err != nil { return nil, fmt.Errorf("%s: %v", InvalidDeploymentErr, err) } - err = v.isTemplateValid(ctx, template) + err = v.isTemplateValid(template) if err != nil { return nil, fmt.Errorf("%s: %v", InvalidDeploymentErr, err) } + err = v.checkComponentsHealth(ctx, template) + if err != nil { + return nil, fmt.Errorf("%s: components verification failed: %v", InvalidDeploymentErr, err) + } return nil, nil } @@ -78,11 +82,11 @@ func (v *DeploymentValidator) ValidateUpdate(ctx context.Context, _ runtime.Obje if !ok { return nil, apierrors.NewBadRequest(fmt.Sprintf("expected Deployment but got a %T", newObj)) } - template, err := v.getDeploymentTemplate(ctx, newDeployment.Spec.Template) + template, err := v.getTemplate(ctx, newDeployment.Spec.Template) if err != nil { return nil, fmt.Errorf("%s: %v", InvalidDeploymentErr, err) } - err = v.isTemplateValid(ctx, template) + err = v.isTemplateValid(template) if err != nil { return nil, fmt.Errorf("%s: %v", InvalidDeploymentErr, err) } @@ -105,11 +109,11 @@ func (v *DeploymentValidator) Default(ctx context.Context, obj runtime.Object) e if deployment.Spec.Config != nil { return nil } - template, err := v.getDeploymentTemplate(ctx, deployment.Spec.Template) + template, err := v.getTemplate(ctx, deployment.Spec.Template) if err != nil { return fmt.Errorf("could not get template for the deployment: %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 *DeploymentValidator) Default(ctx context.Context, obj runtime.Object) e return nil } -func (v *DeploymentValidator) getDeploymentTemplate(ctx context.Context, templateName string) (*v1alpha1.Template, error) { +func (v *DeploymentValidator) getTemplate(ctx context.Context, templateName string) (*v1alpha1.Template, error) { template := &v1alpha1.Template{} templateRef := types.NamespacedName{Name: templateName, Namespace: v1alpha1.TemplatesNamespace} if err := v.Get(ctx, templateRef, template); err != nil { @@ -130,22 +134,18 @@ func (v *DeploymentValidator) getDeploymentTemplate(ctx context.Context, templat return template, nil } -func (v *DeploymentValidator) isTemplateValid(ctx context.Context, template *v1alpha1.Template) error { +func (v *DeploymentValidator) 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 *DeploymentValidator) verifyProviders(ctx context.Context, template *v1alpha1.Template) error { - requiredProviders := template.Status.Providers +func (v *DeploymentValidator) checkComponentsHealth(ctx context.Context, deploymentTemplate *v1alpha1.Template) error { + requiredProviders := deploymentTemplate.Status.Providers management := &v1alpha1.Management{} managementRef := types.NamespacedName{Name: v1alpha1.ManagementName, Namespace: v1alpha1.ManagementNamespace} if err := v.Get(ctx, managementRef, management); err != nil { @@ -153,34 +153,68 @@ func (v *DeploymentValidator) verifyProviders(ctx context.Context, template *v1a } 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/deployment_webhook_test.go b/internal/webhook/deployment_webhook_test.go index 3262aad9..5ef27d6b 100644 --- a/internal/webhook/deployment_webhook_test.go +++ b/internal/webhook/deployment_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" @@ -33,6 +34,8 @@ import ( var ( testTemplateName = "template-test" + newTemplateName = "template-test-2" + capzTemplateName = "cluster-api-provider-azure" mgmt = management.NewManagement( management.WithAvailableProviders(v1alpha1.Providers{ @@ -40,23 +43,37 @@ var ( BootstrapProviders: []string{"k0s"}, ControlPlaneProviders: []string{"k0s"}, }), + management.WithComponentsStatus(map[string]v1alpha1.ComponentStatus{ + v1alpha1.DefaultCoreHMCTemplate: {Success: true}, + v1alpha1.DefaultCoreCAPITemplate: {Success: true}, + }), ) +) + +func TestDeploymentValidateCreateOrUpdate(t *testing.T) { + g := NewWithT(t) - createAndUpdateTests = []struct { + ctx := context.Background() + + tests := []struct { name string - deployment *v1alpha1.Deployment + operation admissionv1.OperationType + oldDeployment *v1alpha1.Deployment + newDeployment *v1alpha1.Deployment existingObjects []runtime.Object err string warnings admission.Warnings }{ { - name: "should fail if the template is unset", - deployment: deployment.NewDeployment(), - err: "the deployment is invalid: templates.hmc.mirantis.com \"\" not found", + name: "create - should fail if the template is unset", + operation: admissionv1.Create, + newDeployment: deployment.NewDeployment(), + err: "the deployment is invalid: templates.hmc.mirantis.com \"\" not found", }, { - name: "should fail if the template is not found in hmc-system namespace", - deployment: deployment.NewDeployment(deployment.WithTemplate(testTemplateName)), + name: "create - should fail if the template is not found in hmc-system namespace", + operation: admissionv1.Create, + newDeployment: deployment.NewDeployment(deployment.WithTemplate(testTemplateName)), existingObjects: []runtime.Object{ mgmt, template.NewTemplate( @@ -67,8 +84,9 @@ var ( err: fmt.Sprintf("the deployment is invalid: templates.hmc.mirantis.com \"%s\" not found", testTemplateName), }, { - name: "should fail if the template was found but is invalid (type is unset)", - deployment: deployment.NewDeployment(deployment.WithTemplate(testTemplateName)), + name: "create - should fail if the template was found but is invalid (type is unset)", + operation: admissionv1.Create, + newDeployment: deployment.NewDeployment(deployment.WithTemplate(testTemplateName)), existingObjects: []runtime.Object{ mgmt, template.NewTemplate(template.WithName(testTemplateName)), @@ -76,8 +94,9 @@ var ( err: "the deployment 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)", - deployment: deployment.NewDeployment(deployment.WithTemplate(testTemplateName)), + name: "create - should fail if the template was found but is invalid (some validation error)", + operation: admissionv1.Create, + newDeployment: deployment.NewDeployment(deployment.WithTemplate(testTemplateName)), existingObjects: []runtime.Object{ mgmt, template.NewTemplate( @@ -92,14 +111,57 @@ var ( err: "the deployment is invalid: the template is not valid: validation error example", }, { - name: "should fail if one or more requested providers are not available yet", - deployment: deployment.NewDeployment(deployment.WithTemplate(testTemplateName)), + name: "create - should fail if one or more requested providers are not available yet", + operation: admissionv1.Create, + newDeployment: deployment.NewDeployment(deployment.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 deployment 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, + newDeployment: deployment.NewDeployment(deployment.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), @@ -112,11 +174,51 @@ var ( template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), ), }, - err: "the deployment 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 deployment 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: "create - should succeed if one or more providers failed but it is not required", + operation: admissionv1.Create, + newDeployment: deployment.NewDeployment(deployment.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: "should succeed", - deployment: deployment.NewDeployment(deployment.WithTemplate(testTemplateName)), + name: "create - should succeed", + operation: admissionv1.Create, + newDeployment: deployment.NewDeployment(deployment.WithTemplate(testTemplateName)), existingObjects: []runtime.Object{ mgmt, template.NewTemplate( @@ -131,44 +233,61 @@ var ( ), }, }, - } -) -func TestDeploymentValidateCreate(t *testing.T) { - g := NewWithT(t) + { + name: "update - should fail if the template is unset", + operation: admissionv1.Update, + oldDeployment: deployment.NewDeployment(deployment.WithTemplate(testTemplateName)), + newDeployment: deployment.NewDeployment(), + err: "the deployment 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, + oldDeployment: deployment.NewDeployment(deployment.WithTemplate(testTemplateName)), + newDeployment: deployment.NewDeployment(deployment.WithTemplate(newTemplateName)), + existingObjects: []runtime.Object{ + mgmt, + template.NewTemplate( + template.WithName(newTemplateName), + template.WithNamespace("default"), + ), + }, + err: fmt.Sprintf("the deployment 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, + oldDeployment: deployment.NewDeployment(deployment.WithTemplate(testTemplateName)), + newDeployment: deployment.NewDeployment(deployment.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 deployment 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 := &DeploymentValidator{Client: c} - warn, err := validator.ValidateCreate(ctx, tt.deployment) - 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.newDeployment) + } else if tt.operation == admissionv1.Update { + warn, err = validator.ValidateUpdate(ctx, tt.oldDeployment, tt.newDeployment) } else { - g.Expect(warn).To(BeEmpty()) + t.Fatalf("Invalid test case, expecting operation [%s,%s], got %s", admissionv1.Create, admissionv1.Update, tt.operation) } - }) - } -} - -func TestDeploymentValidateUpdate(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 := &DeploymentValidator{Client: c} - warn, err := validator.ValidateUpdate(ctx, deployment.NewDeployment(), tt.deployment) if tt.err != "" { g.Expect(err).To(HaveOccurred()) if err.Error() != tt.err {