Skip to content

Commit 63d8e3b

Browse files
anik120anik120
authored
(catsrc) set status reason/message on incorrect polling interval (#2447)
* (catsrc) set status reason/message on incorrect polling interval This PR sets the status reason as InvalidIntervalError and a status message if updateStrategy.RegistryPoll.ParsingError is set for the catsrc. Signed-off-by: Anik Bhattacharjee <[email protected]> * add test to verify error clears on correct update Signed-off-by: Anik Bhattacharjee <[email protected]>
1 parent 3f07f21 commit 63d8e3b

File tree

2 files changed

+147
-62
lines changed

2 files changed

+147
-62
lines changed

‎pkg/controller/operators/catalog/operator.go

+9-4
Original file line numberDiff line numberDiff line change
@@ -727,10 +727,15 @@ func (o *Operator) syncRegistryServer(logger *logrus.Entry, in *v1alpha1.Catalog
727727

728728
// requeue the catalog sync based on the polling interval, for accurate syncs of catalogs with polling enabled
729729
if out.Spec.UpdateStrategy != nil {
730-
logger.Debugf("requeuing registry server sync based on polling interval %s", out.Spec.UpdateStrategy.Interval.Duration.String())
731-
resyncPeriod := reconciler.SyncRegistryUpdateInterval(out, time.Now())
732-
o.catsrcQueueSet.RequeueAfter(out.GetNamespace(), out.GetName(), queueinformer.ResyncWithJitter(resyncPeriod, 0.1)())
733-
return
730+
if out.Spec.UpdateStrategy.RegistryPoll != nil {
731+
if out.Spec.UpdateStrategy.RegistryPoll.ParsingError != "" && out.Status.Reason != v1alpha1.CatalogSourceIntervalInvalidError {
732+
out.SetError(v1alpha1.CatalogSourceIntervalInvalidError, fmt.Errorf(out.Spec.UpdateStrategy.RegistryPoll.ParsingError))
733+
}
734+
logger.Debugf("requeuing registry server sync based on polling interval %s", out.Spec.UpdateStrategy.Interval.Duration.String())
735+
resyncPeriod := reconciler.SyncRegistryUpdateInterval(out, time.Now())
736+
o.catsrcQueueSet.RequeueAfter(out.GetNamespace(), out.GetName(), queueinformer.ResyncWithJitter(resyncPeriod, 0.1)())
737+
return
738+
}
734739
}
735740

736741
if err := o.sources.Remove(sourceKey); err != nil {

‎test/e2e/catalog_e2e_test.go

+138-58
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ const (
4242
catsrcImage = "docker://quay.io/olmtest/catsrc-update-test:"
4343
)
4444

45-
var _ = Describe("Catalog represents a store of bundles which OLM can use to install Operators", func() {
45+
var _ = Describe("Starting CatalogSource e2e tests", func() {
4646
var (
4747
c operatorclient.ClientInterface
4848
crc versioned.Interface
@@ -1074,76 +1074,156 @@ var _ = Describe("Catalog represents a store of bundles which OLM can use to ins
10741074
Expect(err).ShouldNot(HaveOccurred())
10751075
Expect(csv.Spec.Replaces).To(Equal("busybox-dependency.v1.0.0"))
10761076
})
1077-
It("registry polls on the correct interval", func() {
1078-
// Create a catalog source with polling enabled
1079-
// Confirm the following
1080-
// a) the new update pod is spun up roughly in line with the registry polling interval
1081-
// b) the update pod is removed quickly when the image is found to not have changed
1082-
// This is more of a behavioral test that ensures the feature is working as designed.
1083-
1084-
c := newKubeClient()
1085-
crc := newCRClient()
1077+
When("A catalogSource is created with correct polling interval", func() {
10861078

1079+
var source *v1alpha1.CatalogSource
1080+
singlePod := podCount(1)
10871081
sourceName := genName("catalog-")
1088-
source := &v1alpha1.CatalogSource{
1089-
TypeMeta: metav1.TypeMeta{
1090-
Kind: v1alpha1.CatalogSourceKind,
1091-
APIVersion: v1alpha1.CatalogSourceCRDAPIVersion,
1092-
},
1093-
ObjectMeta: metav1.ObjectMeta{
1094-
Name: sourceName,
1095-
Namespace: ns.GetName(),
1096-
Labels: map[string]string{"olm.catalogSource": sourceName},
1097-
},
1098-
Spec: v1alpha1.CatalogSourceSpec{
1099-
SourceType: v1alpha1.SourceTypeGrpc,
1100-
Image: "quay.io/olmtest/catsrc-update-test:new",
1101-
UpdateStrategy: &v1alpha1.UpdateStrategy{
1102-
RegistryPoll: &v1alpha1.RegistryPoll{
1103-
RawInterval: "45s",
1082+
1083+
BeforeEach(func() {
1084+
source = &v1alpha1.CatalogSource{
1085+
TypeMeta: metav1.TypeMeta{
1086+
Kind: v1alpha1.CatalogSourceKind,
1087+
APIVersion: v1alpha1.CatalogSourceCRDAPIVersion,
1088+
},
1089+
ObjectMeta: metav1.ObjectMeta{
1090+
Name: sourceName,
1091+
Namespace: testNamespace,
1092+
Labels: map[string]string{"olm.catalogSource": sourceName},
1093+
},
1094+
Spec: v1alpha1.CatalogSourceSpec{
1095+
SourceType: v1alpha1.SourceTypeGrpc,
1096+
Image: "quay.io/olmtest/catsrc-update-test:new",
1097+
UpdateStrategy: &v1alpha1.UpdateStrategy{
1098+
RegistryPoll: &v1alpha1.RegistryPoll{
1099+
RawInterval: "45s",
1100+
},
11041101
},
11051102
},
1106-
},
1107-
}
1103+
}
11081104

1109-
source, err := crc.OperatorsV1alpha1().CatalogSources(source.GetNamespace()).Create(context.Background(), source, metav1.CreateOptions{})
1110-
Expect(err).ToNot(HaveOccurred())
1105+
source, err := crc.OperatorsV1alpha1().CatalogSources(source.GetNamespace()).Create(context.TODO(), source, metav1.CreateOptions{})
1106+
Expect(err).ToNot(HaveOccurred())
11111107

1112-
// wait for new catalog source pod to be created and report ready
1113-
selector := labels.SelectorFromSet(map[string]string{"olm.catalogSource": source.GetName()})
1114-
singlePod := podCount(1)
1115-
catalogPods, err := awaitPods(GinkgoT(), c, source.GetNamespace(), selector.String(), singlePod)
1116-
Expect(err).ToNot(HaveOccurred())
1117-
Expect(catalogPods).ToNot(BeNil())
1108+
// wait for new catalog source pod to be created and report ready
1109+
selector := labels.SelectorFromSet(map[string]string{"olm.catalogSource": source.GetName()})
11181110

1119-
Eventually(func() (bool, error) {
1120-
podList, err := c.KubernetesInterface().CoreV1().Pods(source.GetNamespace()).List(context.Background(), metav1.ListOptions{LabelSelector: selector.String()})
1121-
if err != nil {
1122-
return false, err
1123-
}
1111+
catalogPods, err := awaitPods(GinkgoT(), c, source.GetNamespace(), selector.String(), singlePod)
1112+
Expect(err).ToNot(HaveOccurred())
1113+
Expect(catalogPods).ToNot(BeNil())
1114+
1115+
Eventually(func() (bool, error) {
1116+
podList, err := c.KubernetesInterface().CoreV1().Pods(source.GetNamespace()).List(context.TODO(), metav1.ListOptions{LabelSelector: selector.String()})
1117+
if err != nil {
1118+
return false, err
1119+
}
11241120

1125-
for _, p := range podList.Items {
1126-
if podReady(&p) {
1127-
return true, nil
1121+
for _, p := range podList.Items {
1122+
if podReady(&p) {
1123+
return true, nil
1124+
}
1125+
return false, nil
11281126
}
1127+
11291128
return false, nil
1130-
}
1129+
}).Should(BeTrue())
1130+
})
11311131

1132-
return false, nil
1133-
}).Should(BeTrue())
1132+
It("registry polls on the correct interval", func() {
1133+
// Wait roughly the polling interval for update pod to show up
1134+
updateSelector := labels.SelectorFromSet(map[string]string{"catalogsource.operators.coreos.com/update": source.GetName()})
1135+
updatePods, err := awaitPodsWithInterval(GinkgoT(), c, source.GetNamespace(), updateSelector.String(), 5*time.Second, 2*time.Minute, singlePod)
1136+
Expect(err).ToNot(HaveOccurred())
1137+
Expect(updatePods).ToNot(BeNil())
1138+
Expect(updatePods.Items).To(HaveLen(1))
1139+
1140+
// No update to image: update pod should be deleted quickly
1141+
noPod := podCount(0)
1142+
updatePods, err = awaitPodsWithInterval(GinkgoT(), c, source.GetNamespace(), updateSelector.String(), 1*time.Second, 30*time.Second, noPod)
1143+
Expect(err).ToNot(HaveOccurred())
1144+
Expect(updatePods.Items).To(HaveLen(0))
1145+
})
11341146

1135-
// Wait roughly the polling interval for update pod to show up
1136-
updateSelector := labels.SelectorFromSet(map[string]string{"catalogsource.operators.coreos.com/update": source.GetName()})
1137-
updatePods, err := awaitPodsWithInterval(GinkgoT(), c, source.GetNamespace(), updateSelector.String(), 5*time.Second, 2*time.Minute, singlePod)
1138-
Expect(err).ToNot(HaveOccurred())
1139-
Expect(updatePods).ToNot(BeNil())
1140-
Expect(updatePods.Items).To(HaveLen(1))
1147+
})
11411148

1142-
// No update to image: update pod should be deleted quickly
1143-
noPod := podCount(0)
1144-
updatePods, err = awaitPodsWithInterval(GinkgoT(), c, source.GetNamespace(), updateSelector.String(), 1*time.Second, 30*time.Second, noPod)
1145-
Expect(err).ToNot(HaveOccurred())
1146-
Expect(updatePods.Items).To(HaveLen(0))
1149+
When("A catalogSource is created with incorrect polling interval", func() {
1150+
1151+
var (
1152+
source *v1alpha1.CatalogSource
1153+
sourceName string
1154+
)
1155+
const (
1156+
incorrectInterval = "45mError.code"
1157+
correctInterval = "45m"
1158+
)
1159+
BeforeEach(func() {
1160+
sourceName = genName("catalog-")
1161+
source = &v1alpha1.CatalogSource{
1162+
TypeMeta: metav1.TypeMeta{
1163+
Kind: v1alpha1.CatalogSourceKind,
1164+
APIVersion: v1alpha1.CatalogSourceCRDAPIVersion,
1165+
},
1166+
ObjectMeta: metav1.ObjectMeta{
1167+
Name: sourceName,
1168+
Namespace: testNamespace,
1169+
Labels: map[string]string{"olm.catalogSource": sourceName},
1170+
},
1171+
Spec: v1alpha1.CatalogSourceSpec{
1172+
SourceType: v1alpha1.SourceTypeGrpc,
1173+
Image: "quay.io/olmtest/catsrc-update-test:new",
1174+
UpdateStrategy: &v1alpha1.UpdateStrategy{
1175+
RegistryPoll: &v1alpha1.RegistryPoll{
1176+
RawInterval: incorrectInterval,
1177+
},
1178+
},
1179+
},
1180+
}
1181+
1182+
_, err := crc.OperatorsV1alpha1().CatalogSources(source.GetNamespace()).Create(context.TODO(), source, metav1.CreateOptions{})
1183+
Expect(err).ToNot(HaveOccurred())
1184+
1185+
})
1186+
AfterEach(func() {
1187+
err := crc.OperatorsV1alpha1().CatalogSources(source.GetNamespace()).Delete(context.TODO(), source.GetName(), metav1.DeleteOptions{})
1188+
Expect(err).ToNot(HaveOccurred())
1189+
})
1190+
It("the catalogsource status communicates that a default interval time is being used instead", func() {
1191+
Eventually(func() bool {
1192+
catsrc, err := crc.OperatorsV1alpha1().CatalogSources(source.GetNamespace()).Get(context.TODO(), source.GetName(), metav1.GetOptions{})
1193+
Expect(err).ToNot(HaveOccurred())
1194+
if catsrc.Status.Reason == v1alpha1.CatalogSourceIntervalInvalidError {
1195+
if catsrc.Status.Message == "error parsing spec.updateStrategy.registryPoll.interval. Using the default value of 15m0s instead. Error: time: unknown unit \"mError\" in duration \"45mError.code\"" {
1196+
return true
1197+
}
1198+
}
1199+
return false
1200+
}).Should(BeTrue())
1201+
})
1202+
When("the catalogsource is updated with a valid polling interval", func() {
1203+
BeforeEach(func() {
1204+
catsrc, err := crc.OperatorsV1alpha1().CatalogSources(source.GetNamespace()).Get(context.TODO(), source.GetName(), metav1.GetOptions{})
1205+
Expect(err).ToNot(HaveOccurred())
1206+
catsrc.Spec.UpdateStrategy.RegistryPoll.RawInterval = correctInterval
1207+
_, err = crc.OperatorsV1alpha1().CatalogSources(catsrc.GetNamespace()).Update(context.TODO(), catsrc, metav1.UpdateOptions{})
1208+
Expect(err).ToNot(HaveOccurred())
1209+
})
1210+
It("the catalogsource spec shows the updated polling interval, and the error message in the status is cleared", func() {
1211+
Eventually(func() error {
1212+
catsrc, err := crc.OperatorsV1alpha1().CatalogSources(source.GetNamespace()).Get(context.TODO(), source.GetName(), metav1.GetOptions{})
1213+
if err != nil {
1214+
return err
1215+
}
1216+
expectedTime, err := time.ParseDuration(correctInterval)
1217+
if err != nil {
1218+
return err
1219+
}
1220+
if catsrc.Status.Reason != "" || (catsrc.Spec.UpdateStrategy.Interval != &metav1.Duration{expectedTime}) {
1221+
return err
1222+
}
1223+
return nil
1224+
}).Should(Succeed())
1225+
})
1226+
})
11471227
})
11481228

11491229
It("adding catalog template adjusts image used", func() {

0 commit comments

Comments
 (0)