Merge pull request #1566 from awgreene/bz-bp

Bug 1827000: Pod Config Deployment Hash Error

Author: openshift-merge-robot

pkg/controller/install/errors.go

@@ -38,11 +38,11 @@ func IsErrorUnrecoverable(err error) bool {
 	if err == nil {
 		return false
 	}
-	_, ok := unrecoverableErrors[reasonForError(err)]
+	_, ok := unrecoverableErrors[ReasonForError(err)]
 	return ok
 }
 
-func reasonForError(err error) string {
+func ReasonForError(err error) string {
 	switch t := err.(type) {
 	case StrategyError:
 		return t.Reason

pkg/controller/operators/olm/operator.go

@@ -1619,7 +1619,11 @@ func (a *Operator) updateInstallStatus(csv *v1alpha1.ClusterServiceVersion, inst
 	}
 
 	if strategyErr != nil {
-		csv.SetPhaseWithEventIfChanged(requeuePhase, requeueConditionReason, fmt.Sprintf("installing: %s", strategyErr), now, a.recorder)
+		if install.ReasonForError(strategyErr) == install.StrategyErrDeploymentUpdated {
+			csv.SetPhaseWithEventIfChanged(v1alpha1.CSVPhaseInstallReady, requeueConditionReason, fmt.Sprintf("installing: %s", strategyErr), now, a.recorder)
+		} else {
+			csv.SetPhaseWithEventIfChanged(requeuePhase, requeueConditionReason, fmt.Sprintf("installing: %s", strategyErr), now, a.recorder)
+		}
 		if err := a.csvQueueSet.Requeue(csv.GetNamespace(), csv.GetName()); err != nil {
 			a.logger.Warn(err.Error())
 		}

pkg/controller/operators/olm/operator_test.go

@@ -8,7 +8,6 @@ import (
 	"crypto/x509"
 	"crypto/x509/pkix"
 	"fmt"
-	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/queueinformer"
 	"math"
 	"math/big"
 	"reflect"
@@ -60,6 +59,7 @@ import (
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorclient"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorlister"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/ownerutil"
+	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/queueinformer"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/scoped"
 )
 
@@ -258,7 +258,7 @@ func NewFakeOperator(ctx context.Context, options ...fakeOperatorOption) (*Opera
 	// Apply options to default config
 	config := &fakeOperatorConfig{
 		operatorConfig: &operatorConfig{
-			resyncPeriod:      queueinformer.ResyncWithJitter(5 * time.Minute, 0.1),
+			resyncPeriod:      queueinformer.ResyncWithJitter(5*time.Minute, 0.1),
 			operatorNamespace: "default",
 			watchedNamespaces: []string{metav1.NamespaceAll},
 			clock:             &utilclock.RealClock{},
@@ -2243,6 +2243,35 @@ func TestTransitionCSV(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "SingleCSVInstallingToInstallReady",
+			initial: initial{
+				csvs: []runtime.Object{
+					csvWithAnnotations(csv("csv1",
+						namespace,
+						"0.0.0",
+						"",
+						installStrategy("csv1-dep1", nil, nil),
+						[]*v1beta1.CustomResourceDefinition{},
+						[]*v1beta1.CustomResourceDefinition{},
+						v1alpha1.CSVPhaseInstalling,
+					), defaultTemplateAnnotations),
+				},
+				clientObjs: []runtime.Object{defaultOperatorGroup},
+				crds:       []runtime.Object{},
+				objs: []runtime.Object{
+					withLabels(
+						deployment("csv1-dep1", namespace, "sa", defaultTemplateAnnotations),
+						map[string]string{install.DeploymentSpecHashLabelKey: "BadHash"},
+					),
+				},
+			},
+			expected: expected{
+				csvStates: map[string]csvState{
+					"csv1": {exists: true, phase: v1alpha1.CSVPhaseInstallReady, reason: "InstallWaiting"},
+				},
+			},
+		},
 		{
 			name: "SingleCSVSucceededToSucceeded/UnmanagedDeploymentInNamespace",
 			initial: initial{

test/e2e/subscription_e2e_test.go

@@ -1353,10 +1353,6 @@ func TestCreateNewSubscriptionWithPodConfig(t *testing.T) {
 	require.NotNil(t, subscription)
 
 	csv, err := fetchCSV(t, crClient, subscription.Status.CurrentCSV, testNamespace, buildCSVConditionChecker(v1alpha1.CSVPhaseSucceeded))
-	if err != nil {
-		// TODO: If OLM doesn't have the subscription in its cache when it initially creates the deployment, the CSV will hang on "Installing" until it reaches the five-minute timeout, then succeed on a retry. It should be possible to skip the wait and retry immediately, but in the meantime, giving this test a little extra patience should mitigate flakes.
-		csv, err = fetchCSV(t, crClient, subscription.Status.CurrentCSV, testNamespace, buildCSVConditionChecker(v1alpha1.CSVPhaseSucceeded))
-	}
 	require.NoError(t, err)
 
 	proxyEnv := proxyEnvVarFunc(t, config)