Add support to check minimum availability time in waitForDeploymentRo…

…llout

pkg/operator/operator_test.go

@@ -25,7 +25,7 @@ import (
 	fakedynamic "k8s.io/client-go/dynamic/fake"
 	"k8s.io/client-go/informers"
 	fakekube "k8s.io/client-go/kubernetes/fake"
-	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
+	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/client-go/util/workqueue"
 )
@@ -40,7 +40,7 @@ const (
 func newFakeOperator(kubeObjects []runtime.Object, osObjects []runtime.Object, stopCh <-chan struct{}) *Operator {
 	kubeClient := fakekube.NewSimpleClientset(kubeObjects...)
 	osClient := fakeos.NewSimpleClientset(osObjects...)
-	dynamicClient := fakedynamic.NewSimpleDynamicClient(clientgoscheme.Scheme, kubeObjects...)
+	dynamicClient := fakedynamic.NewSimpleDynamicClient(scheme.Scheme, kubeObjects...)
 	kubeNamespacedSharedInformer := informers.NewSharedInformerFactoryWithOptions(kubeClient, 2*time.Minute, informers.WithNamespace(targetNamespace))
 	configSharedInformer := configinformersv1.NewSharedInformerFactoryWithOptions(osClient, 2*time.Minute)
 	featureGateInformer := configSharedInformer.Config().V1().FeatureGates()

pkg/operator/sync.go

@@ -9,6 +9,7 @@ import (
 	"github.com/openshift/library-go/pkg/operator/resource/resourceapply"
 	"github.com/openshift/library-go/pkg/operator/resource/resourcemerge"
 	machinecontroller "github.com/openshift/machine-api-operator/pkg/controller/machine"
+	"github.com/openshift/machine-api-operator/pkg/util/conditions"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -19,11 +20,12 @@ import (
 )
 
 const (
-	deploymentRolloutPollInterval = time.Second
-	deploymentRolloutTimeout      = 5 * time.Minute
-	daemonsetRolloutPollInterval  = time.Second
-	daemonsetRolloutTimeout       = 5 * time.Minute
-	machineAPITerminationHandler  = "machine-api-termination-handler"
+	deploymentRolloutPollInterval     = time.Second
+	deploymentRolloutTimeout          = 5 * time.Minute
+	deploymentMinimumAvailabilityTime = 3 * time.Minute
+	daemonsetRolloutPollInterval      = time.Second
+	daemonsetRolloutTimeout           = 5 * time.Minute
+	machineAPITerminationHandler      = "machine-api-termination-handler"
 )
 
 func (optr *Operator) syncAll(config *OperatorConfig) error {
@@ -84,7 +86,7 @@ func (optr *Operator) syncClusterAPIController(config *OperatorConfig) error {
 	}
 	if updated {
 		resourcemerge.SetDeploymentGeneration(&optr.generations, d)
-		err := optr.waitForDeploymentRollout(controllersDeployment)
+		err := optr.waitForDeploymentRollout(controllersDeployment, deploymentRolloutPollInterval, deploymentRolloutTimeout)
 		if err != nil {
 			return err
 		}
@@ -136,17 +138,19 @@ func (optr *Operator) syncBaremetalControllers(config *OperatorConfig) error {
 	}
 	if updated {
 		resourcemerge.SetDeploymentGeneration(&optr.generations, d)
-		return optr.waitForDeploymentRollout(metal3Deployment)
+		return optr.waitForDeploymentRollout(metal3Deployment, deploymentRolloutPollInterval, deploymentRolloutTimeout)
 	}
 
 	return nil
 }
 
-func (optr *Operator) waitForDeploymentRollout(resource *appsv1.Deployment) error {
+func (optr *Operator) waitForDeploymentRollout(resource *appsv1.Deployment, pollInterval, rolloutTimeout time.Duration) error {
 	var lastError error
-	err := wait.Poll(deploymentRolloutPollInterval, deploymentRolloutTimeout, func() (bool, error) {
+	err := wait.Poll(pollInterval, rolloutTimeout, func() (bool, error) {
 		d, err := optr.deployLister.Deployments(resource.Namespace).Get(resource.Name)
 		if apierrors.IsNotFound(err) {
+			lastError = fmt.Errorf("deployment %s is not found", d.GetName())
+			glog.Error(lastError)
 			return false, nil
 		}
 		if err != nil {
@@ -163,9 +167,27 @@ func (optr *Operator) waitForDeploymentRollout(resource *appsv1.Deployment) erro
 		}
 
 		if d.Generation <= d.Status.ObservedGeneration && d.Status.UpdatedReplicas == d.Status.Replicas && d.Status.UnavailableReplicas == 0 {
+			c := conditions.GetDeploymentCondition(d, appsv1.DeploymentAvailable)
+			if c == nil {
+				lastError = fmt.Errorf("deployment %s is not reporting available yet", d.GetName())
+				glog.V(4).Info(lastError)
+				return false, nil
+			}
+			if c.Status == corev1.ConditionFalse {
+				lastError = fmt.Errorf("deployment %s is reporting available=false", d.GetName())
+				glog.V(4).Info(lastError)
+				return false, nil
+			}
+			if c.LastTransitionTime.Time.Add(deploymentMinimumAvailabilityTime).After(time.Now()) {
+				lastError = fmt.Errorf("deployment %s has been available for less than 3 min", d.GetName())
+				glog.V(4).Info(lastError)
+				return false, nil
+			}
+
 			lastError = nil
 			return true, nil
 		}
+
 		lastError = fmt.Errorf("deployment %s is not ready. status: (replicas: %d, updated: %d, ready: %d, unavailable: %d)", d.Name, d.Status.Replicas, d.Status.UpdatedReplicas, d.Status.ReadyReplicas, d.Status.UnavailableReplicas)
 		glog.V(4).Info(lastError)
 		return false, nil

pkg/operator/sync_test.go

@@ -0,0 +1,120 @@
+package operator
+
+import (
+	"fmt"
+	"testing"
+	"time"
+
+	appsv1 "k8s.io/api/apps/v1"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+)
+
+func TestWaitForDeploymentRollout(t *testing.T) {
+	testCases := []struct {
+		name       string
+		deployment *appsv1.Deployment
+		expected   error
+	}{
+		{
+			name: "Deployment is available for more than deploymentMinimumAvailabilityTime min",
+			deployment: &appsv1.Deployment{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "Deployment",
+					APIVersion: "apps/v1",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test",
+					Namespace: targetNamespace,
+				},
+				Status: appsv1.DeploymentStatus{
+					Replicas:            1,
+					UpdatedReplicas:     1,
+					ReadyReplicas:       1,
+					AvailableReplicas:   1,
+					UnavailableReplicas: 0,
+					Conditions: []appsv1.DeploymentCondition{
+						{
+							Type:               appsv1.DeploymentAvailable,
+							Status:             corev1.ConditionTrue,
+							LastTransitionTime: metav1.Time{Time: time.Now().Add(-deploymentMinimumAvailabilityTime - 1*time.Second)},
+						},
+					},
+				},
+			},
+			expected: nil,
+		},
+		{
+			name: "Deployment is available for less than deploymentMinimumAvailabilityTime min",
+			deployment: &appsv1.Deployment{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "Deployment",
+					APIVersion: "apps/v1",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test",
+					Namespace: targetNamespace,
+				},
+				Status: appsv1.DeploymentStatus{
+					Replicas:            1,
+					UpdatedReplicas:     1,
+					ReadyReplicas:       1,
+					AvailableReplicas:   1,
+					UnavailableReplicas: 0,
+					Conditions: []appsv1.DeploymentCondition{
+						{
+							Type:               appsv1.DeploymentAvailable,
+							Status:             corev1.ConditionTrue,
+							LastTransitionTime: metav1.Time{Time: time.Now().Add(-10 * time.Second)},
+						},
+					},
+				},
+			},
+			expected: fmt.Errorf("deployment test has been available for less than 3 min"),
+		},
+		{
+			name: "Deployment has unavailable replicas",
+			deployment: &appsv1.Deployment{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "Deployment",
+					APIVersion: "apps/v1",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test",
+					Namespace: targetNamespace,
+				},
+				Status: appsv1.DeploymentStatus{
+					Replicas:            1,
+					UpdatedReplicas:     1,
+					ReadyReplicas:       1,
+					AvailableReplicas:   1,
+					UnavailableReplicas: 1,
+					Conditions: []appsv1.DeploymentCondition{
+						{
+							Type:               appsv1.DeploymentAvailable,
+							Status:             corev1.ConditionTrue,
+							LastTransitionTime: metav1.Time{Time: time.Now().Add(-10 * time.Second)},
+						},
+					},
+				},
+			},
+			expected: fmt.Errorf("deployment test is not ready. status: (replicas: 1, updated: 1, ready: 1, unavailable: 1)"),
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			optr := newFakeOperator([]runtime.Object{tc.deployment}, nil, make(<-chan struct{}))
+
+			got := optr.waitForDeploymentRollout(tc.deployment, 1*time.Second, 3*time.Second)
+			if tc.expected != nil {
+				if tc.expected.Error() != got.Error() {
+					t.Errorf("Got: %v, expected: %v", got, tc.expected)
+				}
+			} else if tc.expected != got {
+				t.Errorf("Got: %v, expected: %v", got, tc.expected)
+			}
+		})
+	}
+}

pkg/util/conditions/conditions.go

@@ -1,6 +1,7 @@
 package conditions
 
 import (
+	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 )
 
@@ -13,3 +14,13 @@ func GetNodeCondition(node *corev1.Node, conditionType corev1.NodeConditionType)
 	}
 	return nil
 }
+
+// GetDeploymentCondition returns node condition by type
+func GetDeploymentCondition(deployment *appsv1.Deployment, conditionType appsv1.DeploymentConditionType) *appsv1.DeploymentCondition {
+	for _, cond := range deployment.Status.Conditions {
+		if cond.Type == conditionType {
+			return &cond
+		}
+	}
+	return nil
+}