diff --git a/pkg/framework/deployment.go b/pkg/framework/deployment.go index 69736fdaf..c6fe06a90 100644 --- a/pkg/framework/deployment.go +++ b/pkg/framework/deployment.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "reflect" + "time" kappsapi "k8s.io/api/apps/v1" "k8s.io/apimachinery/pkg/types" @@ -12,15 +13,15 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -// GetDeployment gets deployment object by name and namespace. -func GetDeployment(c client.Client, name, namespace string) (*kappsapi.Deployment, error) { +// GetDeployment gets deployment object by name and namespace with custom timeout +func GetDeployment(c client.Client, name, namespace string, timeout time.Duration) (*kappsapi.Deployment, error) { key := types.NamespacedName{ Namespace: namespace, Name: name, } d := &kappsapi.Deployment{} - if err := wait.PollImmediate(RetryShort, WaitShort, func() (bool, error) { + if err := wait.PollImmediate(RetryShort, timeout, func() (bool, error) { if err := c.Get(context.TODO(), key, d); err != nil { klog.Errorf("Error querying api for Deployment object %q: %v, retrying...", name, err) return false, nil @@ -32,9 +33,9 @@ func GetDeployment(c client.Client, name, namespace string) (*kappsapi.Deploymen return d, nil } -// DeleteDeployment deletes the specified deployment -func DeleteDeployment(c client.Client, deployment *kappsapi.Deployment) error { - return wait.PollImmediate(RetryShort, WaitShort, func() (bool, error) { +// DeleteDeployment deletes the specified deployment with custom timeout +func DeleteDeployment(c client.Client, deployment *kappsapi.Deployment, timeout time.Duration) error { + return wait.PollImmediate(RetryShort, timeout, func() (bool, error) { if err := c.Delete(context.TODO(), deployment); err != nil { klog.Errorf("error querying api for deployment object %q: %v, retrying...", deployment.Name, err) return false, nil @@ -43,10 +44,11 @@ func DeleteDeployment(c client.Client, deployment *kappsapi.Deployment) error { }) } -// UpdateDeployment updates the specified deployment -func UpdateDeployment(c client.Client, name, namespace string, updated *kappsapi.Deployment) error { - return wait.PollImmediate(RetryShort, WaitMedium, func() (bool, error) { - d, err := GetDeployment(c, name, namespace) +// UpdateDeployment updates the specified deployment with custom timeout +func UpdateDeployment(c client.Client, name, namespace string, + updated *kappsapi.Deployment, timeout time.Duration) error { + return wait.PollImmediate(RetryShort, timeout, func() (bool, error) { + d, err := GetDeployment(c, name, namespace, WaitShort) if err != nil { klog.Errorf("Error getting deployment: %v", err) return false, nil @@ -59,10 +61,10 @@ func UpdateDeployment(c client.Client, name, namespace string, updated *kappsapi }) } -// IsDeploymentAvailable returns true if the deployment has one or more availabe replicas -func IsDeploymentAvailable(c client.Client, name, namespace string) bool { - if err := wait.PollImmediate(RetryShort, WaitLong, func() (bool, error) { - d, err := GetDeployment(c, name, namespace) +// IsDeploymentAvailable returns true if the deployment has one or more availabe replicas with custom timeout +func IsDeploymentAvailable(c client.Client, name, namespace string, timeout time.Duration) bool { + if err := wait.PollImmediate(RetryShort, timeout, func() (bool, error) { + d, err := GetDeployment(c, name, namespace, WaitShort) if err != nil { klog.Errorf("Error getting deployment: %v", err) return false, nil @@ -82,9 +84,11 @@ func IsDeploymentAvailable(c client.Client, name, namespace string) bool { return true } -// IsDeploymentSynced returns true if provided deployment spec matched one found on cluster -func IsDeploymentSynced(c client.Client, dep *kappsapi.Deployment, name, namespace string) bool { - d, err := GetDeployment(c, name, namespace) +// IsDeploymentSynced returns true if provided deployment spec +// matched one found on cluster with custom timeout +func IsDeploymentSynced(c client.Client, dep *kappsapi.Deployment, + name, namespace string, timeout time.Duration) bool { + d, err := GetDeployment(c, name, namespace, timeout) if err != nil { klog.Errorf("Error getting deployment: %v", err) return false diff --git a/pkg/framework/webhooks.go b/pkg/framework/webhooks.go index 07c2b9c1d..36d606bfa 100644 --- a/pkg/framework/webhooks.go +++ b/pkg/framework/webhooks.go @@ -3,6 +3,7 @@ package framework import ( "context" "fmt" + "time" mapiv1 "github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1" admissionregistrationv1 "k8s.io/api/admissionregistration/v1" @@ -19,12 +20,18 @@ var DefaultValidatingWebhookConfiguration = mapiv1.NewValidatingWebhookConfigura // DefaultMutatingWebhookConfiguration is a default mutating webhook configuration resource provided by MAO var DefaultMutatingWebhookConfiguration = mapiv1.NewMutatingWebhookConfiguration() +// Webhooks may take up to 10 minutes to sync. When changed we first have to wait for a full Deployment +// rollout (5 minutes) and DaemonSet rollout (another 5 minutes) to occur before they are synced. Then the +// process starts all over again. This applies to all webhook operations. +// TODO: Make MAO resource state management unblocking +var waitWebhooks = 10 * time.Minute + // GetMutatingWebhookConfiguration gets MutatingWebhookConfiguration object by name func GetMutatingWebhookConfiguration(c client.Client, name string) (*admissionregistrationv1.MutatingWebhookConfiguration, error) { key := client.ObjectKey{Name: name} existing := &admissionregistrationv1.MutatingWebhookConfiguration{} - if err := wait.PollImmediate(RetryShort, WaitShort, func() (bool, error) { + if err := wait.PollImmediate(RetryShort, waitWebhooks, func() (bool, error) { if err := c.Get(context.TODO(), key, existing); err != nil { klog.Errorf("Error querying api for MutatingWebhookConfiguration object %q: %v, retrying...", name, err) return false, nil @@ -41,7 +48,7 @@ func GetValidatingWebhookConfiguration(c client.Client, name string) (*admission key := client.ObjectKey{Name: name} existing := &admissionregistrationv1.ValidatingWebhookConfiguration{} - if err := wait.PollImmediate(RetryShort, WaitShort, func() (bool, error) { + if err := wait.PollImmediate(RetryShort, waitWebhooks, func() (bool, error) { if err := c.Get(context.TODO(), key, existing); err != nil { klog.Errorf("Error querying api for ValidatingWebhookConfiguration object %q: %v, retrying...", name, err) return false, nil @@ -55,7 +62,7 @@ func GetValidatingWebhookConfiguration(c client.Client, name string) (*admission // DeleteValidatingWebhookConfiguration deletes the specified ValidatingWebhookConfiguration object func DeleteValidatingWebhookConfiguration(c client.Client, webhookConfiguraiton *admissionregistrationv1.ValidatingWebhookConfiguration) error { - return wait.PollImmediate(RetryShort, WaitShort, func() (bool, error) { + return wait.PollImmediate(RetryShort, waitWebhooks, func() (bool, error) { if err := c.Delete(context.TODO(), webhookConfiguraiton); apierrors.IsNotFound(err) { return true, nil } else if err != nil { @@ -68,7 +75,7 @@ func DeleteValidatingWebhookConfiguration(c client.Client, webhookConfiguraiton // DeleteMutatingWebhookConfiguration deletes the specified MutatingWebhookConfiguration object func DeleteMutatingWebhookConfiguration(c client.Client, webhookConfiguraiton *admissionregistrationv1.MutatingWebhookConfiguration) error { - return wait.PollImmediate(RetryShort, WaitShort, func() (bool, error) { + return wait.PollImmediate(RetryShort, waitWebhooks, func() (bool, error) { if err := c.Delete(context.TODO(), webhookConfiguraiton); apierrors.IsNotFound(err) { return true, nil } else if err != nil { @@ -81,7 +88,7 @@ func DeleteMutatingWebhookConfiguration(c client.Client, webhookConfiguraiton *a // UpdateMutatingWebhookConfiguration updates the specified mutating webhook configuration func UpdateMutatingWebhookConfiguration(c client.Client, updated *admissionregistrationv1.MutatingWebhookConfiguration) error { - return wait.PollImmediate(RetryShort, WaitShort, func() (bool, error) { + return wait.PollImmediate(RetryShort, waitWebhooks, func() (bool, error) { existing, err := GetMutatingWebhookConfiguration(c, updated.Name) if err != nil { klog.Errorf("Error getting MutatingWebhookConfiguration: %v", err) @@ -97,7 +104,7 @@ func UpdateMutatingWebhookConfiguration(c client.Client, updated *admissionregis // UpdateValidatingWebhookConfiguration updates the specified mutating webhook configuration func UpdateValidatingWebhookConfiguration(c client.Client, updated *admissionregistrationv1.ValidatingWebhookConfiguration) error { - return wait.PollImmediate(RetryShort, WaitShort, func() (bool, error) { + return wait.PollImmediate(RetryShort, waitWebhooks, func() (bool, error) { existing, err := GetValidatingWebhookConfiguration(c, updated.Name) if err != nil { klog.Errorf("Error getting ValidatingWebhookConfiguration: %v", err) @@ -113,7 +120,7 @@ func UpdateValidatingWebhookConfiguration(c client.Client, updated *admissionreg // IsMutatingWebhookConfigurationSynced expects a matching MutatingWebhookConfiguration to be present in the cluster func IsMutatingWebhookConfigurationSynced(c client.Client) bool { - if err := wait.PollImmediate(RetryShort, WaitMedium, func() (bool, error) { + if err := wait.PollImmediate(RetryShort, waitWebhooks, func() (bool, error) { existing, err := GetMutatingWebhookConfiguration(c, DefaultMutatingWebhookConfiguration.Name) if err != nil { klog.Errorf("Error getting MutatingWebhookConfiguration: %v", err) @@ -137,7 +144,7 @@ func IsMutatingWebhookConfigurationSynced(c client.Client) bool { // IsValidatingWebhookConfigurationSynced expects a matching MutatingWebhookConfiguration to be present in the cluster func IsValidatingWebhookConfigurationSynced(c client.Client) bool { - if err := wait.PollImmediate(RetryShort, WaitMedium, func() (bool, error) { + if err := wait.PollImmediate(RetryShort, waitWebhooks, func() (bool, error) { existing, err := GetValidatingWebhookConfiguration(c, DefaultValidatingWebhookConfiguration.Name) if err != nil { klog.Errorf("Error getting MutatingWebhookConfiguration: %v", err) diff --git a/pkg/infra/spot.go b/pkg/infra/spot.go index ae650a1e7..cfe2e342e 100644 --- a/pkg/infra/spot.go +++ b/pkg/infra/spot.go @@ -149,7 +149,8 @@ var _ = Describe("[Feature:Machines] Running on Spot", func() { Expect(client.Create(ctx, deployment)).To(Succeed()) delObjects[deployment.Name] = deployment - Expect(framework.IsDeploymentAvailable(client, deployment.Name, deployment.Namespace)).To(BeTrue()) + Expect(framework.IsDeploymentAvailable(client, deployment.Name, + deployment.Namespace, framework.WaitLong)).To(BeTrue()) }) var machine *mapiv1.Machine diff --git a/pkg/operators/cluster-autoscaler-operator.go b/pkg/operators/cluster-autoscaler-operator.go index b6db4238a..aaa9bf383 100644 --- a/pkg/operators/cluster-autoscaler-operator.go +++ b/pkg/operators/cluster-autoscaler-operator.go @@ -81,7 +81,8 @@ var _ = Describe("[Feature:Operators] Cluster autoscaler operator deployment sho var err error client, err := framework.LoadClient() Expect(err).NotTo(HaveOccurred()) - Expect(framework.IsDeploymentAvailable(client, "cluster-autoscaler-operator", framework.MachineAPINamespace)).To(BeTrue()) + Expect(framework.IsDeploymentAvailable(client, "cluster-autoscaler-operator", + framework.MachineAPINamespace, framework.WaitLong)).To(BeTrue()) }) }) diff --git a/pkg/operators/cluster-machine-approver.go b/pkg/operators/cluster-machine-approver.go index 0b19a0630..640b08fbb 100644 --- a/pkg/operators/cluster-machine-approver.go +++ b/pkg/operators/cluster-machine-approver.go @@ -17,7 +17,8 @@ var _ = Describe("[Feature:Operators] Cluster Machine Approver deployment", func client, err := framework.LoadClient() Expect(err).NotTo(HaveOccurred()) - Expect(framework.IsDeploymentAvailable(client, cmaDeployment, cmaNamespace)).To(BeTrue()) + Expect(framework.IsDeploymentAvailable(client, cmaDeployment, + cmaNamespace, framework.WaitLong)).To(BeTrue()) }) }) diff --git a/pkg/operators/machine-api-operator.go b/pkg/operators/machine-api-operator.go index 20ff15ff4..0d5a19185 100644 --- a/pkg/operators/machine-api-operator.go +++ b/pkg/operators/machine-api-operator.go @@ -2,11 +2,13 @@ package operators import ( "fmt" + "time" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "github.com/openshift/cluster-api-actuator-pkg/pkg/framework" "k8s.io/utils/pointer" + "sigs.k8s.io/controller-runtime/pkg/client" ) var ( @@ -14,95 +16,80 @@ var ( maoManagedDeployment = "machine-api-controllers" ) +// Webhooks may take up to 10 minutes to sync. When changed we first have to wait for a full Deployment +// rollout (5 minutes) and DaemonSet rollout (another 5 minutes) to occur before they are synced. +var defaultReconcileTime = 10 * time.Minute + var _ = Describe("[Feature:Operators] Machine API operator deployment should", func() { defer GinkgoRecover() + var client client.Client - It("be available", func() { - client, err := framework.LoadClient() + BeforeEach(func() { + var err error + client, err = framework.LoadClient() Expect(err).NotTo(HaveOccurred()) - Expect(framework.IsDeploymentAvailable(client, maoDeployment, framework.MachineAPINamespace)).To(BeTrue()) + By(fmt.Sprintf("checking deployment %q is available", maoDeployment)) + Expect(framework.IsDeploymentAvailable(client, maoDeployment, + framework.MachineAPINamespace, framework.WaitLong)).To(BeTrue()) }) It("reconcile controllers deployment", func() { - client, err := framework.LoadClient() - Expect(err).NotTo(HaveOccurred()) - - initialDeployment, err := framework.GetDeployment(client, maoManagedDeployment, framework.MachineAPINamespace) + initialDeployment, err := framework.GetDeployment(client, maoManagedDeployment, framework.MachineAPINamespace, defaultReconcileTime) Expect(err).NotTo(HaveOccurred()) By(fmt.Sprintf("checking deployment %q is available", maoManagedDeployment)) - Expect(framework.IsDeploymentAvailable(client, maoManagedDeployment, framework.MachineAPINamespace)).To(BeTrue()) + Expect(framework.IsDeploymentAvailable(client, maoManagedDeployment, framework.MachineAPINamespace, defaultReconcileTime)).To(BeTrue()) By(fmt.Sprintf("deleting deployment %q", maoManagedDeployment)) - Expect(framework.DeleteDeployment(client, initialDeployment)).NotTo(HaveOccurred()) + Expect(framework.DeleteDeployment(client, initialDeployment, defaultReconcileTime)).NotTo(HaveOccurred()) By(fmt.Sprintf("checking deployment %q is available again", maoManagedDeployment)) - Expect(framework.IsDeploymentAvailable(client, maoManagedDeployment, framework.MachineAPINamespace)).To(BeTrue()) + Expect(framework.IsDeploymentAvailable(client, maoManagedDeployment, framework.MachineAPINamespace, defaultReconcileTime)).To(BeTrue()) By(fmt.Sprintf("checking deployment %q spec matches", maoManagedDeployment)) - Expect(framework.IsDeploymentSynced(client, initialDeployment, maoManagedDeployment, framework.MachineAPINamespace)).To(BeTrue()) + Expect(framework.IsDeploymentSynced(client, initialDeployment, maoManagedDeployment, framework.MachineAPINamespace, defaultReconcileTime)).To(BeTrue()) }) It("maintains deployment spec", func() { - client, err := framework.LoadClient() - Expect(err).NotTo(HaveOccurred()) - - initialDeployment, err := framework.GetDeployment(client, maoManagedDeployment, framework.MachineAPINamespace) + initialDeployment, err := framework.GetDeployment(client, maoManagedDeployment, framework.MachineAPINamespace, defaultReconcileTime) Expect(err).NotTo(HaveOccurred()) By(fmt.Sprintf("checking deployment %q is available", maoManagedDeployment)) - Expect(framework.IsDeploymentAvailable(client, maoManagedDeployment, framework.MachineAPINamespace)).To(BeTrue()) + Expect(framework.IsDeploymentAvailable(client, maoManagedDeployment, framework.MachineAPINamespace, defaultReconcileTime)).To(BeTrue()) changedDeployment := initialDeployment.DeepCopy() changedDeployment.Spec.Replicas = pointer.Int32Ptr(0) By(fmt.Sprintf("updating deployment %q", maoManagedDeployment)) - Expect(framework.UpdateDeployment(client, maoManagedDeployment, framework.MachineAPINamespace, changedDeployment)).NotTo(HaveOccurred()) + Expect(framework.UpdateDeployment(client, maoManagedDeployment, framework.MachineAPINamespace, changedDeployment, defaultReconcileTime)).NotTo(HaveOccurred()) By(fmt.Sprintf("checking deployment %q spec matches", maoManagedDeployment)) - Expect(framework.IsDeploymentSynced(client, initialDeployment, maoManagedDeployment, framework.MachineAPINamespace)).To(BeTrue()) + Expect(framework.IsDeploymentSynced(client, initialDeployment, maoManagedDeployment, framework.MachineAPINamespace, defaultReconcileTime)).To(BeTrue()) By(fmt.Sprintf("checking deployment %q is available again", maoManagedDeployment)) - Expect(framework.IsDeploymentAvailable(client, maoManagedDeployment, framework.MachineAPINamespace)).To(BeTrue()) + Expect(framework.IsDeploymentAvailable(client, maoManagedDeployment, framework.MachineAPINamespace, defaultReconcileTime)).To(BeTrue()) }) It("reconcile mutating webhook configuration", func() { - client, err := framework.LoadClient() - Expect(err).NotTo(HaveOccurred()) - Expect(framework.IsMutatingWebhookConfigurationSynced(client)).To(BeTrue()) }) It("reconcile validating webhook configuration", func() { - client, err := framework.LoadClient() - Expect(err).NotTo(HaveOccurred()) - Expect(framework.IsValidatingWebhookConfigurationSynced(client)).To(BeTrue()) }) It("recover after validating webhook configuration deletion", func() { - client, err := framework.LoadClient() - Expect(err).NotTo(HaveOccurred()) - Expect(framework.DeleteValidatingWebhookConfiguration(client, framework.DefaultValidatingWebhookConfiguration)).To(Succeed()) - Expect(framework.IsValidatingWebhookConfigurationSynced(client)).To(BeTrue()) }) It("recover after mutating webhook configuration deletion", func() { - client, err := framework.LoadClient() - Expect(err).NotTo(HaveOccurred()) - Expect(framework.DeleteMutatingWebhookConfiguration(client, framework.DefaultMutatingWebhookConfiguration)).To(Succeed()) - Expect(framework.IsMutatingWebhookConfigurationSynced(client)).To(BeTrue()) }) It("maintains spec after mutating webhook configuration change and preserve caBundle", func() { - client, err := framework.LoadClient() - Expect(err).NotTo(HaveOccurred()) - initial, err := framework.GetMutatingWebhookConfiguration(client, framework.DefaultMutatingWebhookConfiguration.Name) Expect(initial).ToNot(BeNil()) Expect(err).To(Succeed()) @@ -127,9 +114,6 @@ var _ = Describe("[Feature:Operators] Machine API operator deployment should", f }) It("maintains spec after validating webhook configuration change and preserve caBundle", func() { - client, err := framework.LoadClient() - Expect(err).NotTo(HaveOccurred()) - initial, err := framework.GetValidatingWebhookConfiguration(client, framework.DefaultValidatingWebhookConfiguration.Name) Expect(initial).ToNot(BeNil()) Expect(err).To(Succeed())