Skip to content

Commit 2294bcc

Browse files
openshift-merge-robotopenshift-merge-robot
authored
Merge pull request #1892 from benluddy/copied-csv-updatetime
Bug 1905599: Preserve original .status.lastUpdateTime in copied CSVs.
2 parents 8860fa5 + 7fc883c commit 2294bcc

File tree

3 files changed

+286
-2
lines changed

3 files changed

+286
-2
lines changed

‎pkg/controller/operators/olm/operatorgroup.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -734,7 +734,6 @@ func (a *Operator) copyToNamespace(csv *v1alpha1.ClusterServiceVersion, namespac
734734
logger.Debug("updating status")
735735
// Must use fetchedCSV because UpdateStatus(...) checks resource UID.
736736
fetchedCSV.Status = newCSV.Status
737-
fetchedCSV.Status.LastUpdateTime = a.now()
738737
if fetchedCSV, err = a.client.OperatorsV1alpha1().ClusterServiceVersions(namespace).UpdateStatus(context.TODO(), fetchedCSV, metav1.UpdateOptions{}); err != nil {
739738
logger.WithError(err).Error("status update for target CSV failed")
740739
return nil, err
@@ -756,7 +755,6 @@ func (a *Operator) copyToNamespace(csv *v1alpha1.ClusterServiceVersion, namespac
756755
}
757756
createdCSV.Status.Reason = v1alpha1.CSVReasonCopied
758757
createdCSV.Status.Message = fmt.Sprintf("The operator is running in %s but is managing this namespace", csv.GetNamespace())
759-
createdCSV.Status.LastUpdateTime = a.now()
760758
if _, err := a.client.OperatorsV1alpha1().ClusterServiceVersions(namespace).UpdateStatus(context.TODO(), createdCSV, metav1.UpdateOptions{}); err != nil {
761759
a.logger.Errorf("Status update for CSV failed: %v", err)
762760
return nil, err

‎pkg/controller/operators/olm/operatorgroup_test.go

Lines changed: 199 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,199 @@
1+
package olm
2+
3+
import (
4+
"io/ioutil"
5+
"testing"
6+
"time"
7+
8+
"github.com/sirupsen/logrus"
9+
"github.com/stretchr/testify/require"
10+
11+
"k8s.io/apimachinery/pkg/api/errors"
12+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
13+
"k8s.io/apimachinery/pkg/labels"
14+
ktesting "k8s.io/client-go/testing"
15+
16+
"github.com/operator-framework/api/pkg/operators/v1alpha1"
17+
"github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/clientset/versioned/fake"
18+
listersv1alpha1 "github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/listers/operators/v1alpha1"
19+
"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorlister/operatorlisterfakes"
20+
)
21+
22+
func TestCopyToNamespace(t *testing.T) {
23+
gvr := v1alpha1.SchemeGroupVersion.WithResource("clusterserviceversions")
24+
25+
for _, tc := range []struct {
26+
Name string
27+
Namespace string
28+
Original *v1alpha1.ClusterServiceVersion
29+
ExistingCopy *v1alpha1.ClusterServiceVersion
30+
ExpectedResult *v1alpha1.ClusterServiceVersion
31+
ExpectedError string
32+
ExpectedActions []ktesting.Action
33+
}{
34+
{
35+
Name: "copy to original namespace returns error",
36+
Namespace: "foo",
37+
Original: &v1alpha1.ClusterServiceVersion{
38+
ObjectMeta: metav1.ObjectMeta{
39+
Namespace: "foo",
40+
},
41+
},
42+
ExpectedError: "bug: can not copy to active namespace foo",
43+
},
44+
{
45+
Name: "status updated if meaningfully changed",
46+
Namespace: "bar",
47+
Original: &v1alpha1.ClusterServiceVersion{
48+
ObjectMeta: metav1.ObjectMeta{
49+
Name: "name",
50+
Namespace: "foo",
51+
},
52+
Status: v1alpha1.ClusterServiceVersionStatus{
53+
LastUpdateTime: &metav1.Time{Time: time.Unix(2, 0)},
54+
},
55+
},
56+
ExistingCopy: &v1alpha1.ClusterServiceVersion{
57+
ObjectMeta: metav1.ObjectMeta{
58+
Name: "name",
59+
Namespace: "bar",
60+
},
61+
Status: v1alpha1.ClusterServiceVersionStatus{
62+
LastUpdateTime: &metav1.Time{Time: time.Unix(1, 0)},
63+
},
64+
},
65+
ExpectedResult: &v1alpha1.ClusterServiceVersion{
66+
ObjectMeta: metav1.ObjectMeta{
67+
Name: "name",
68+
Namespace: "bar",
69+
},
70+
Status: v1alpha1.ClusterServiceVersionStatus{
71+
LastUpdateTime: &metav1.Time{Time: time.Unix(2, 0)},
72+
Message: "The operator is running in foo but is managing this namespace",
73+
Reason: v1alpha1.CSVReasonCopied,
74+
},
75+
},
76+
ExpectedActions: []ktesting.Action{
77+
ktesting.NewUpdateSubresourceAction(gvr, "status", "bar", &v1alpha1.ClusterServiceVersion{
78+
ObjectMeta: metav1.ObjectMeta{
79+
Name: "name",
80+
Namespace: "bar",
81+
},
82+
Status: v1alpha1.ClusterServiceVersionStatus{
83+
LastUpdateTime: &metav1.Time{Time: time.Unix(2, 0)},
84+
Message: "The operator is running in foo but is managing this namespace",
85+
Reason: v1alpha1.CSVReasonCopied,
86+
},
87+
}),
88+
},
89+
},
90+
{
91+
Name: "status not updated if not meaningfully changed",
92+
Namespace: "bar",
93+
Original: &v1alpha1.ClusterServiceVersion{
94+
ObjectMeta: metav1.ObjectMeta{
95+
Name: "name",
96+
Namespace: "foo",
97+
},
98+
Status: v1alpha1.ClusterServiceVersionStatus{
99+
LastUpdateTime: &metav1.Time{Time: time.Unix(2, 0)},
100+
},
101+
},
102+
ExistingCopy: &v1alpha1.ClusterServiceVersion{
103+
ObjectMeta: metav1.ObjectMeta{
104+
Name: "name",
105+
Namespace: "bar",
106+
},
107+
Status: v1alpha1.ClusterServiceVersionStatus{
108+
LastUpdateTime: &metav1.Time{Time: time.Unix(2, 0)},
109+
Message: "The operator is running in foo but is managing this namespace",
110+
Reason: v1alpha1.CSVReasonCopied},
111+
},
112+
ExpectedResult: &v1alpha1.ClusterServiceVersion{
113+
ObjectMeta: metav1.ObjectMeta{
114+
Name: "name",
115+
Namespace: "bar",
116+
},
117+
Status: v1alpha1.ClusterServiceVersionStatus{
118+
LastUpdateTime: &metav1.Time{Time: time.Unix(2, 0)},
119+
Message: "The operator is running in foo but is managing this namespace",
120+
Reason: v1alpha1.CSVReasonCopied,
121+
},
122+
},
123+
},
124+
} {
125+
t.Run(tc.Name, func(t *testing.T) {
126+
lister := &operatorlisterfakes.FakeOperatorLister{}
127+
v1alpha1lister := &operatorlisterfakes.FakeOperatorsV1alpha1Lister{}
128+
lister.OperatorsV1alpha1Returns(v1alpha1lister)
129+
130+
client := fake.NewSimpleClientset()
131+
if tc.ExistingCopy != nil {
132+
client = fake.NewSimpleClientset(tc.ExistingCopy)
133+
v1alpha1lister.ClusterServiceVersionListerReturns(FakeClusterServiceVersionLister{tc.ExistingCopy})
134+
}
135+
136+
logger := logrus.New()
137+
logger.SetOutput(ioutil.Discard)
138+
139+
o := &Operator{
140+
lister: lister,
141+
client: client,
142+
logger: logger,
143+
}
144+
result, err := o.copyToNamespace(tc.Original, tc.Namespace)
145+
146+
require := require.New(t)
147+
if tc.ExpectedError == "" {
148+
require.NoError(err)
149+
} else {
150+
require.EqualError(err, tc.ExpectedError)
151+
}
152+
require.Equal(tc.ExpectedResult, result)
153+
154+
actions := client.Actions()
155+
if len(actions) == 0 {
156+
actions = nil
157+
}
158+
require.Equal(tc.ExpectedActions, actions)
159+
})
160+
}
161+
}
162+
163+
type FakeClusterServiceVersionLister []*v1alpha1.ClusterServiceVersion
164+
165+
func (l FakeClusterServiceVersionLister) List(selector labels.Selector) ([]*v1alpha1.ClusterServiceVersion, error) {
166+
var result []*v1alpha1.ClusterServiceVersion
167+
for _, csv := range l {
168+
if !selector.Matches(labels.Set(csv.GetLabels())) {
169+
continue
170+
}
171+
result = append(result, csv)
172+
}
173+
return result, nil
174+
}
175+
176+
func (l FakeClusterServiceVersionLister) ClusterServiceVersions(namespace string) listersv1alpha1.ClusterServiceVersionNamespaceLister {
177+
var filtered []*v1alpha1.ClusterServiceVersion
178+
for _, csv := range l {
179+
if csv.GetNamespace() != namespace {
180+
continue
181+
}
182+
filtered = append(filtered, csv)
183+
}
184+
return FakeClusterServiceVersionLister(filtered)
185+
}
186+
187+
func (l FakeClusterServiceVersionLister) Get(name string) (*v1alpha1.ClusterServiceVersion, error) {
188+
for _, csv := range l {
189+
if csv.GetName() == name {
190+
return csv, nil
191+
}
192+
}
193+
return nil, errors.NewNotFound(v1alpha1.Resource("clusterserviceversion"), name)
194+
}
195+
196+
var (
197+
_ listersv1alpha1.ClusterServiceVersionLister = FakeClusterServiceVersionLister{}
198+
_ listersv1alpha1.ClusterServiceVersionNamespaceLister = FakeClusterServiceVersionLister{}
199+
)

‎test/e2e/csv_e2e_test.go

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"k8s.io/apimachinery/pkg/util/wait"
2424
"k8s.io/apimachinery/pkg/watch"
2525
apiregistrationv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1"
26+
"sigs.k8s.io/controller-runtime/pkg/client"
2627

2728
v1 "github.com/operator-framework/api/pkg/operators/v1"
2829
"github.com/operator-framework/api/pkg/operators/v1alpha1"
@@ -47,6 +48,92 @@ var _ = Describe("ClusterServiceVersion", func() {
4748
TearDown(testNamespace)
4849
})
4950

51+
When("a copied csv exists", func() {
52+
var (
53+
target corev1.Namespace
54+
original v1alpha1.ClusterServiceVersion
55+
copy v1alpha1.ClusterServiceVersion
56+
)
57+
58+
BeforeEach(func() {
59+
target = corev1.Namespace{
60+
ObjectMeta: metav1.ObjectMeta{
61+
GenerateName: "watched-",
62+
},
63+
}
64+
Expect(ctx.Ctx().Client().Create(context.Background(), &target)).To(Succeed())
65+
66+
original = v1alpha1.ClusterServiceVersion{
67+
TypeMeta: metav1.TypeMeta{
68+
Kind: v1alpha1.ClusterServiceVersionKind,
69+
APIVersion: v1alpha1.ClusterServiceVersionAPIVersion,
70+
},
71+
ObjectMeta: metav1.ObjectMeta{
72+
GenerateName: "csv-",
73+
Namespace: testNamespace,
74+
},
75+
Spec: v1alpha1.ClusterServiceVersionSpec{
76+
InstallStrategy: newNginxInstallStrategy(genName("csv-"), nil, nil),
77+
InstallModes: []v1alpha1.InstallMode{
78+
{
79+
Type: v1alpha1.InstallModeTypeAllNamespaces,
80+
Supported: true,
81+
},
82+
},
83+
},
84+
}
85+
Expect(ctx.Ctx().Client().Create(context.Background(), &original)).To(Succeed())
86+
87+
Eventually(func() error {
88+
key, err := client.ObjectKeyFromObject(&original)
89+
Expect(err).ToNot(HaveOccurred())
90+
key.Namespace = target.GetName()
91+
return ctx.Ctx().Client().Get(context.Background(), key, &copy)
92+
}).Should(Succeed())
93+
})
94+
95+
AfterEach(func() {
96+
if target.GetName() != "" {
97+
Expect(ctx.Ctx().Client().Delete(context.Background(), &target)).To(Succeed())
98+
}
99+
})
100+
101+
It("is synchronized with the original csv", func() {
102+
Eventually(func() error {
103+
key, err := client.ObjectKeyFromObject(&copy)
104+
if err != nil {
105+
return err
106+
}
107+
108+
key.Namespace = target.Name
109+
if err := ctx.Ctx().Client().Get(context.Background(), key, &copy); err != nil {
110+
return err
111+
}
112+
113+
copy.Status.LastUpdateTime = &metav1.Time{Time: time.Unix(1, 0)}
114+
return ctx.Ctx().Client().Status().Update(context.Background(), &copy)
115+
}).Should(Succeed())
116+
117+
Eventually(func() (bool, error) {
118+
key, err := client.ObjectKeyFromObject(&original)
119+
if err != nil {
120+
return false, err
121+
}
122+
123+
if err := ctx.Ctx().Client().Get(context.Background(), key, &original); err != nil {
124+
return false, err
125+
}
126+
127+
key.Namespace = target.Name
128+
if err := ctx.Ctx().Client().Get(context.Background(), key, &copy); err != nil {
129+
return false, err
130+
}
131+
132+
return original.Status.LastUpdateTime.Equal(copy.Status.LastUpdateTime), nil
133+
}).Should(BeTrue(), "Change to status of copy should have been reverted")
134+
})
135+
})
136+
50137
It("create with unmet requirements min kube version", func() {
51138

52139
depName := genName("dep-")

0 commit comments

Comments
 (0)