Bug 1905599: Preserve original .status.lastUpdateTime in copied CSVs.

pkg/controller/operators/olm/operatorgroup.go

@@ -734,7 +734,6 @@ func (a *Operator) copyToNamespace(csv *v1alpha1.ClusterServiceVersion, namespac
 			logger.Debug("updating status")
 			// Must use fetchedCSV because UpdateStatus(...) checks resource UID.
 			fetchedCSV.Status = newCSV.Status
-			fetchedCSV.Status.LastUpdateTime = a.now()
 			if fetchedCSV, err = a.client.OperatorsV1alpha1().ClusterServiceVersions(namespace).UpdateStatus(context.TODO(), fetchedCSV, metav1.UpdateOptions{}); err != nil {
 				logger.WithError(err).Error("status update for target CSV failed")
 				return nil, err
@@ -756,7 +755,6 @@ func (a *Operator) copyToNamespace(csv *v1alpha1.ClusterServiceVersion, namespac
 		}
 		createdCSV.Status.Reason = v1alpha1.CSVReasonCopied
 		createdCSV.Status.Message = fmt.Sprintf("The operator is running in %s but is managing this namespace", csv.GetNamespace())
-		createdCSV.Status.LastUpdateTime = a.now()
 		if _, err := a.client.OperatorsV1alpha1().ClusterServiceVersions(namespace).UpdateStatus(context.TODO(), createdCSV, metav1.UpdateOptions{}); err != nil {
 			a.logger.Errorf("Status update for CSV failed: %v", err)
 			return nil, err

pkg/controller/operators/olm/operatorgroup_test.go

@@ -0,0 +1,199 @@
+package olm
+
+import (
+	"io/ioutil"
+	"testing"
+	"time"
+
+	"github.com/sirupsen/logrus"
+	"github.com/stretchr/testify/require"
+
+	"k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
+	ktesting "k8s.io/client-go/testing"
+
+	"github.com/operator-framework/api/pkg/operators/v1alpha1"
+	"github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/clientset/versioned/fake"
+	listersv1alpha1 "github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/listers/operators/v1alpha1"
+	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorlister/operatorlisterfakes"
+)
+
+func TestCopyToNamespace(t *testing.T) {
+	gvr := v1alpha1.SchemeGroupVersion.WithResource("clusterserviceversions")
+
+	for _, tc := range []struct {
+		Name            string
+		Namespace       string
+		Original        *v1alpha1.ClusterServiceVersion
+		ExistingCopy    *v1alpha1.ClusterServiceVersion
+		ExpectedResult  *v1alpha1.ClusterServiceVersion
+		ExpectedError   string
+		ExpectedActions []ktesting.Action
+	}{
+		{
+			Name:      "copy to original namespace returns error",
+			Namespace: "foo",
+			Original: &v1alpha1.ClusterServiceVersion{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "foo",
+				},
+			},
+			ExpectedError: "bug: can not copy to active namespace foo",
+		},
+		{
+			Name:      "status updated if meaningfully changed",
+			Namespace: "bar",
+			Original: &v1alpha1.ClusterServiceVersion{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "name",
+					Namespace: "foo",
+				},
+				Status: v1alpha1.ClusterServiceVersionStatus{
+					LastUpdateTime: &metav1.Time{Time: time.Unix(2, 0)},
+				},
+			},
+			ExistingCopy: &v1alpha1.ClusterServiceVersion{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "name",
+					Namespace: "bar",
+				},
+				Status: v1alpha1.ClusterServiceVersionStatus{
+					LastUpdateTime: &metav1.Time{Time: time.Unix(1, 0)},
+				},
+			},
+			ExpectedResult: &v1alpha1.ClusterServiceVersion{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "name",
+					Namespace: "bar",
+				},
+				Status: v1alpha1.ClusterServiceVersionStatus{
+					LastUpdateTime: &metav1.Time{Time: time.Unix(2, 0)},
+					Message:        "The operator is running in foo but is managing this namespace",
+					Reason:         v1alpha1.CSVReasonCopied,
+				},
+			},
+			ExpectedActions: []ktesting.Action{
+				ktesting.NewUpdateSubresourceAction(gvr, "status", "bar", &v1alpha1.ClusterServiceVersion{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "name",
+						Namespace: "bar",
+					},
+					Status: v1alpha1.ClusterServiceVersionStatus{
+						LastUpdateTime: &metav1.Time{Time: time.Unix(2, 0)},
+						Message:        "The operator is running in foo but is managing this namespace",
+						Reason:         v1alpha1.CSVReasonCopied,
+					},
+				}),
+			},
+		},
+		{
+			Name:      "status not updated if not meaningfully changed",
+			Namespace: "bar",
+			Original: &v1alpha1.ClusterServiceVersion{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "name",
+					Namespace: "foo",
+				},
+				Status: v1alpha1.ClusterServiceVersionStatus{
+					LastUpdateTime: &metav1.Time{Time: time.Unix(2, 0)},
+				},
+			},
+			ExistingCopy: &v1alpha1.ClusterServiceVersion{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "name",
+					Namespace: "bar",
+				},
+				Status: v1alpha1.ClusterServiceVersionStatus{
+					LastUpdateTime: &metav1.Time{Time: time.Unix(2, 0)},
+					Message:        "The operator is running in foo but is managing this namespace",
+					Reason:         v1alpha1.CSVReasonCopied},
+			},
+			ExpectedResult: &v1alpha1.ClusterServiceVersion{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "name",
+					Namespace: "bar",
+				},
+				Status: v1alpha1.ClusterServiceVersionStatus{
+					LastUpdateTime: &metav1.Time{Time: time.Unix(2, 0)},
+					Message:        "The operator is running in foo but is managing this namespace",
+					Reason:         v1alpha1.CSVReasonCopied,
+				},
+			},
+		},
+	} {
+		t.Run(tc.Name, func(t *testing.T) {
+			lister := &operatorlisterfakes.FakeOperatorLister{}
+			v1alpha1lister := &operatorlisterfakes.FakeOperatorsV1alpha1Lister{}
+			lister.OperatorsV1alpha1Returns(v1alpha1lister)
+
+			client := fake.NewSimpleClientset()
+			if tc.ExistingCopy != nil {
+				client = fake.NewSimpleClientset(tc.ExistingCopy)
+				v1alpha1lister.ClusterServiceVersionListerReturns(FakeClusterServiceVersionLister{tc.ExistingCopy})
+			}
+
+			logger := logrus.New()
+			logger.SetOutput(ioutil.Discard)
+
+			o := &Operator{
+				lister: lister,
+				client: client,
+				logger: logger,
+			}
+			result, err := o.copyToNamespace(tc.Original, tc.Namespace)
+
+			require := require.New(t)
+			if tc.ExpectedError == "" {
+				require.NoError(err)
+			} else {
+				require.EqualError(err, tc.ExpectedError)
+			}
+			require.Equal(tc.ExpectedResult, result)
+
+			actions := client.Actions()
+			if len(actions) == 0 {
+				actions = nil
+			}
+			require.Equal(tc.ExpectedActions, actions)
+		})
+	}
+}
+
+type FakeClusterServiceVersionLister []*v1alpha1.ClusterServiceVersion
+
+func (l FakeClusterServiceVersionLister) List(selector labels.Selector) ([]*v1alpha1.ClusterServiceVersion, error) {
+	var result []*v1alpha1.ClusterServiceVersion
+	for _, csv := range l {
+		if !selector.Matches(labels.Set(csv.GetLabels())) {
+			continue
+		}
+		result = append(result, csv)
+	}
+	return result, nil
+}
+
+func (l FakeClusterServiceVersionLister) ClusterServiceVersions(namespace string) listersv1alpha1.ClusterServiceVersionNamespaceLister {
+	var filtered []*v1alpha1.ClusterServiceVersion
+	for _, csv := range l {
+		if csv.GetNamespace() != namespace {
+			continue
+		}
+		filtered = append(filtered, csv)
+	}
+	return FakeClusterServiceVersionLister(filtered)
+}
+
+func (l FakeClusterServiceVersionLister) Get(name string) (*v1alpha1.ClusterServiceVersion, error) {
+	for _, csv := range l {
+		if csv.GetName() == name {
+			return csv, nil
+		}
+	}
+	return nil, errors.NewNotFound(v1alpha1.Resource("clusterserviceversion"), name)
+}
+
+var (
+	_ listersv1alpha1.ClusterServiceVersionLister          = FakeClusterServiceVersionLister{}
+	_ listersv1alpha1.ClusterServiceVersionNamespaceLister = FakeClusterServiceVersionLister{}
+)

test/e2e/csv_e2e_test.go

@@ -23,6 +23,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/apimachinery/pkg/watch"
 	apiregistrationv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	v1 "github.com/operator-framework/api/pkg/operators/v1"
 	"github.com/operator-framework/api/pkg/operators/v1alpha1"
@@ -47,6 +48,92 @@ var _ = Describe("ClusterServiceVersion", func() {
 		TearDown(testNamespace)
 	})
 
+	When("a copied csv exists", func() {
+		var (
+			target   corev1.Namespace
+			original v1alpha1.ClusterServiceVersion
+			copy     v1alpha1.ClusterServiceVersion
+		)
+
+		BeforeEach(func() {
+			target = corev1.Namespace{
+				ObjectMeta: metav1.ObjectMeta{
+					GenerateName: "watched-",
+				},
+			}
+			Expect(ctx.Ctx().Client().Create(context.Background(), &target)).To(Succeed())
+
+			original = v1alpha1.ClusterServiceVersion{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       v1alpha1.ClusterServiceVersionKind,
+					APIVersion: v1alpha1.ClusterServiceVersionAPIVersion,
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					GenerateName: "csv-",
+					Namespace:    testNamespace,
+				},
+				Spec: v1alpha1.ClusterServiceVersionSpec{
+					InstallStrategy: newNginxInstallStrategy(genName("csv-"), nil, nil),
+					InstallModes: []v1alpha1.InstallMode{
+						{
+							Type:      v1alpha1.InstallModeTypeAllNamespaces,
+							Supported: true,
+						},
+					},
+				},
+			}
+			Expect(ctx.Ctx().Client().Create(context.Background(), &original)).To(Succeed())
+
+			Eventually(func() error {
+				key, err := client.ObjectKeyFromObject(&original)
+				Expect(err).ToNot(HaveOccurred())
+				key.Namespace = target.GetName()
+				return ctx.Ctx().Client().Get(context.Background(), key, &copy)
+			}).Should(Succeed())
+		})
+
+		AfterEach(func() {
+			if target.GetName() != "" {
+				Expect(ctx.Ctx().Client().Delete(context.Background(), &target)).To(Succeed())
+			}
+		})
+
+		It("is synchronized with the original csv", func() {
+			Eventually(func() error {
+				key, err := client.ObjectKeyFromObject(&copy)
+				if err != nil {
+					return err
+				}
+
+				key.Namespace = target.Name
+				if err := ctx.Ctx().Client().Get(context.Background(), key, &copy); err != nil {
+					return err
+				}
+
+				copy.Status.LastUpdateTime = &metav1.Time{Time: time.Unix(1, 0)}
+				return ctx.Ctx().Client().Status().Update(context.Background(), &copy)
+			}).Should(Succeed())
+
+			Eventually(func() (bool, error) {
+				key, err := client.ObjectKeyFromObject(&original)
+				if err != nil {
+					return false, err
+				}
+
+				if err := ctx.Ctx().Client().Get(context.Background(), key, &original); err != nil {
+					return false, err
+				}
+
+				key.Namespace = target.Name
+				if err := ctx.Ctx().Client().Get(context.Background(), key, &copy); err != nil {
+					return false, err
+				}
+
+				return original.Status.LastUpdateTime.Equal(copy.Status.LastUpdateTime), nil
+			}).Should(BeTrue(), "Change to status of copy should have been reverted")
+		})
+	})
+
 	It("create with unmet requirements min kube version", func() {
 
 		depName := genName("dep-")