Skip to content

Commit 930163c

Browse files
dinhxuanvudinhxuanvu
committed
Add a new test case to cover preexisting CRD case
Signed-off-by: Vu Dinh <[email protected]>
1 parent 24c59dc commit 930163c

File tree

3 files changed

+213
-57
lines changed

3 files changed

+213
-57
lines changed

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

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -102,17 +102,18 @@ func NewOperator(ctx context.Context, kubeconfigPath string, clock utilclock.Clo
102102

103103
// Allocate the new instance of an Operator.
104104
op := &Operator{
105-
Operator: queueOperator,
106-
logger: logger,
107-
clock: clock,
108-
opClient: opClient,
109-
client: crClient,
110-
lister: lister,
111-
namespace: operatorNamespace,
112-
sources: make(map[resolver.CatalogKey]resolver.SourceRef),
113-
resolver: resolver.NewOperatorsV1alpha1Resolver(lister),
114-
catsrcQueueSet: queueinformer.NewEmptyResourceQueueSet(),
115-
subQueueSet: queueinformer.NewEmptyResourceQueueSet(),
105+
Operator: queueOperator,
106+
logger: logger,
107+
clock: clock,
108+
opClient: opClient,
109+
client: crClient,
110+
lister: lister,
111+
namespace: operatorNamespace,
112+
sources: make(map[resolver.CatalogKey]resolver.SourceRef),
113+
resolver: resolver.NewOperatorsV1alpha1Resolver(lister),
114+
catsrcQueueSet: queueinformer.NewEmptyResourceQueueSet(),
115+
subQueueSet: queueinformer.NewEmptyResourceQueueSet(),
116+
csvProvidedAPIsIndexer: map[string]cache.Indexer{},
116117
}
117118
op.reconciler = reconciler.NewRegistryReconcilerFactory(lister, opClient, configmapRegistryImage, op.now)
118119

@@ -253,19 +254,18 @@ func NewOperator(ctx context.Context, kubeconfigPath string, clock utilclock.Clo
253254
}
254255

255256
// Register CustomResourceDefinition QueueInformer
256-
customResourceDefinitionInformer := extinf.NewSharedInformerFactory(op.OpClient.ApiextensionsV1beta1Interface(), wakeupInterval).Apiextensions().V1beta1().CustomResourceDefinitions()
257-
op.RegisterQueueInformer(queueinformer.NewInformer(
258-
workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "customresourcedefinitions"),
259-
customResourceDefinitionInformer.Informer(),
260-
op.syncObject,
261-
&cache.ResourceEventHandlerFuncs{
262-
DeleteFunc: op.handleDeletion,
263-
},
264-
"customresourcedefinitions",
265-
metrics.NewMetricsNil(),
266-
logger,
267-
))
268-
op.lister.APIExtensionsV1beta1().RegisterCustomResourceDefinitionLister(customResourceDefinitionInformer.Lister())
257+
crdInformer := extinf.NewSharedInformerFactory(op.opClient.ApiextensionsV1beta1Interface(), resyncPeriod).Apiextensions().V1beta1().CustomResourceDefinitions()
258+
op.lister.APIExtensionsV1beta1().RegisterCustomResourceDefinitionLister(crdInformer.Lister())
259+
crdQueueInformer, err := queueinformer.NewQueueInformer(
260+
ctx,
261+
queueinformer.WithLogger(op.logger),
262+
queueinformer.WithInformer(crdInformer.Informer()),
263+
queueinformer.WithSyncer(queueinformer.LegacySyncHandler(op.syncObject).ToSyncerWithDelete(op.handleDeletion)),
264+
)
265+
if err != nil {
266+
return nil, err
267+
}
268+
op.RegisterQueueInformer(crdQueueInformer)
269269

270270
// Namespace sync for resolving subscriptions
271271
namespaceInformer := informers.NewSharedInformerFactory(op.opClient.KubernetesInterface(), resyncPeriod).Core().V1().Namespaces()
@@ -1070,7 +1070,7 @@ func (o *Operator) ExecutePlan(plan *v1alpha1.InstallPlan) error {
10701070
if len(matchedCSV) == 1 {
10711071
// Attempt to update CRD
10721072
crd.SetResourceVersion(currentCRD.GetResourceVersion())
1073-
_, err = o.OpClient.ApiextensionsV1beta1Interface().ApiextensionsV1beta1().CustomResourceDefinitions().Update(&crd)
1073+
_, err = o.opClient.ApiextensionsV1beta1Interface().ApiextensionsV1beta1().CustomResourceDefinitions().Update(&crd)
10741074
if err != nil {
10751075
return errorwrap.Wrapf(err, "error update CRD: %s", step.Resource.Name)
10761076
}

‎pkg/lib/index/api.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,13 +48,13 @@ func CRDProviderNames(indexers map[string]cache.Indexer, crd v1beta1ext.CustomRe
4848
crdSpec[fmt.Sprintf("%s/%s/%s", crd.Spec.Group, crd.Spec.Version, crd.Spec.Names.Kind)] = struct{}{}
4949
}
5050
for _, indexer := range indexers {
51-
for key, _ := range crdSpec {
51+
for key := range crdSpec {
5252
csvs, err := indexer.ByIndex(ProvidedAPIsIndexFuncKey, key)
5353
if err != nil {
5454
return nil, err
5555
}
56-
for _, csv := range csvs {
57-
csv, ok := csv.(*v1alpha1.ClusterServiceVersion)
56+
for _, item := range csvs {
57+
csv, ok := item.(*v1alpha1.ClusterServiceVersion)
5858
if !ok {
5959
continue
6060
}

‎test/e2e/installplan_e2e_test.go

Lines changed: 185 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -647,15 +647,15 @@ func TestUpdateInstallPlan(t *testing.T) {
647647
t.Run("UpdateSingleExistingCRDOwner", func(t *testing.T) {
648648
defer cleaner.NotifyTestComplete(t, true)
649649

650-
mainPackageName := genName("nginx-")
650+
mainPackageName := genName("nginx-update-")
651651

652652
mainPackageStable := fmt.Sprintf("%s-stable", mainPackageName)
653653

654654
stableChannel := "stable"
655655

656656
mainNamedStrategy := newNginxInstallStrategy(genName("dep-"), nil, nil)
657657

658-
crdPlural := genName("ins-")
658+
crdPlural := genName("ins-update-")
659659
crdName := crdPlural + ".cluster.com"
660660
mainCRD := apiextensions.CustomResourceDefinition{
661661
ObjectMeta: metav1.ObjectMeta{
@@ -708,16 +708,6 @@ func TestUpdateInstallPlan(t *testing.T) {
708708
},
709709
}
710710

711-
expectedCRDVersions := map[v1beta1.CustomResourceDefinitionVersion]struct{}{}
712-
for _, version := range updatedCRD.Spec.Versions {
713-
key := v1beta1.CustomResourceDefinitionVersion{
714-
Name: version.Name,
715-
Served: version.Served,
716-
Storage: version.Storage,
717-
}
718-
expectedCRDVersions[key] = struct{}{}
719-
}
720-
721711
mainCSV := newCSV(mainPackageStable, testNamespace, "", semver.MustParse("0.1.0"), []apiextensions.CustomResourceDefinition{mainCRD}, nil, mainNamedStrategy)
722712

723713
c := newKubeClient(t)
@@ -726,7 +716,7 @@ func TestUpdateInstallPlan(t *testing.T) {
726716
require.NoError(t, crc.OperatorsV1alpha1().Subscriptions(testNamespace).DeleteCollection(&metav1.DeleteOptions{}, metav1.ListOptions{}))
727717
}()
728718

729-
mainCatalogName := genName("mock-ocs-main-")
719+
mainCatalogName := genName("mock-ocs-main-update-")
730720

731721
// Create separate manifests for each CatalogSource
732722
mainManifests := []registry.PackageManifest{
@@ -746,7 +736,7 @@ func TestUpdateInstallPlan(t *testing.T) {
746736
_, err := fetchCatalogSource(t, crc, mainCatalogName, testNamespace, catalogSourceRegistryPodSynced)
747737
require.NoError(t, err)
748738

749-
subscriptionName := genName("sub-nginx-")
739+
subscriptionName := genName("sub-nginx-update-")
750740
subscriptionCleanup := createSubscriptionForCatalog(t, crc, testNamespace, subscriptionName, mainCatalogName, mainPackageName, stableChannel, "", v1alpha1.ApprovalAutomatic)
751741
defer subscriptionCleanup()
752742

@@ -775,47 +765,213 @@ func TestUpdateInstallPlan(t *testing.T) {
775765
_, err = awaitCSV(t, crc, testNamespace, mainCSV.GetName(), csvAnyChecker)
776766
require.NoError(t, err)
777767

778-
// Create new CSV to replace the one CSV
779-
updatedCSV := newCSV(mainPackageStable+"-v2", testNamespace, mainPackageStable, semver.MustParse("0.1.1"), []apiextensions.CustomResourceDefinition{mainCRD}, nil, mainNamedStrategy)
768+
updateInternalCatalog(t, c, crc, mainCatalogName, testNamespace, []apiextensions.CustomResourceDefinition{updatedCRD}, []v1alpha1.ClusterServiceVersion{mainCSV}, mainManifests)
769+
770+
// Update the subscription resource
771+
err = crc.OperatorsV1alpha1().Subscriptions(testNamespace).DeleteCollection(metav1.NewDeleteOptions(0), metav1.ListOptions{})
772+
require.NoError(t, err)
773+
774+
// existing cleanup should remove this
775+
createSubscriptionForCatalog(t, crc, testNamespace, subscriptionName, mainCatalogName, mainPackageName, stableChannel, "", v1alpha1.ApprovalAutomatic)
776+
777+
// Wait for subscription to update
778+
updatedSubscription, err := fetchSubscription(t, crc, testNamespace, subscriptionName, subscriptionHasInstallPlanChecker)
779+
require.NoError(t, err)
780+
781+
// Verify installplan created and installed
782+
fetchedUpdatedInstallPlan, err := fetchInstallPlan(t, crc, updatedSubscription.Status.InstallPlanRef.Name, buildInstallPlanPhaseCheckFunc(v1alpha1.InstallPlanPhaseComplete))
783+
require.NoError(t, err)
784+
require.NotEqual(t, fetchedInstallPlan.GetName(), fetchedUpdatedInstallPlan.GetName())
785+
786+
// Wait for csv to update
787+
_, err = awaitCSV(t, crc, testNamespace, mainCSV.GetName(), csvAnyChecker)
788+
require.NoError(t, err)
789+
790+
// Get the CRD to see if it is updated
791+
fetchedCRD, err := c.ApiextensionsV1beta1Interface().ApiextensionsV1beta1().CustomResourceDefinitions().Get(crdName, metav1.GetOptions{})
792+
require.NoError(t, err)
793+
require.Equal(t, len(fetchedCRD.Spec.Versions), len(updatedCRD.Spec.Versions), "The CRD versions counts don't match")
794+
795+
fetchedCRDVersions := map[v1beta1.CustomResourceDefinitionVersion]struct{}{}
796+
for _, version := range fetchedCRD.Spec.Versions {
797+
key := v1beta1.CustomResourceDefinitionVersion{
798+
Name: version.Name,
799+
Served: version.Served,
800+
Storage: version.Storage,
801+
}
802+
fetchedCRDVersions[key] = struct{}{}
803+
}
804+
805+
for _, version := range updatedCRD.Spec.Versions {
806+
key := v1beta1.CustomResourceDefinitionVersion{
807+
Name: version.Name,
808+
Served: version.Served,
809+
Storage: version.Storage,
810+
}
811+
_, ok := fetchedCRDVersions[key]
812+
require.True(t, ok, "couldn't find %v in fetched CRD versions: %#v", key, fetchedCRDVersions)
813+
}
814+
})
815+
816+
t.Run("UpdatePreexistingCRDFailed", func(t *testing.T) {
817+
defer cleaner.NotifyTestComplete(t, true)
818+
819+
c := newKubeClient(t)
820+
crc := newCRClient(t)
821+
defer func() {
822+
require.NoError(t, crc.OperatorsV1alpha1().Subscriptions(testNamespace).DeleteCollection(&metav1.DeleteOptions{}, metav1.ListOptions{}))
823+
}()
824+
825+
mainPackageName := genName("nginx-update2-")
826+
827+
mainPackageStable := fmt.Sprintf("%s-stable", mainPackageName)
828+
829+
stableChannel := "stable"
830+
831+
mainNamedStrategy := newNginxInstallStrategy(genName("dep-"), nil, nil)
832+
833+
crdPlural := genName("ins-update2-")
834+
crdName := crdPlural + ".cluster.com"
835+
mainCRD := apiextensions.CustomResourceDefinition{
836+
ObjectMeta: metav1.ObjectMeta{
837+
Name: crdName,
838+
},
839+
Spec: apiextensions.CustomResourceDefinitionSpec{
840+
Group: "cluster.com",
841+
Versions: []apiextensions.CustomResourceDefinitionVersion{
842+
{
843+
Name: "v1alpha1",
844+
Served: true,
845+
Storage: true,
846+
},
847+
},
848+
Names: apiextensions.CustomResourceDefinitionNames{
849+
Plural: crdPlural,
850+
Singular: crdPlural,
851+
Kind: crdPlural,
852+
ListKind: "list" + crdPlural,
853+
},
854+
Scope: "Namespaced",
855+
},
856+
}
857+
858+
updatedCRD := apiextensions.CustomResourceDefinition{
859+
ObjectMeta: metav1.ObjectMeta{
860+
Name: crdName,
861+
},
862+
Spec: apiextensions.CustomResourceDefinitionSpec{
863+
Group: "cluster.com",
864+
Versions: []apiextensions.CustomResourceDefinitionVersion{
865+
{
866+
Name: "v1alpha1",
867+
Served: true,
868+
Storage: true,
869+
},
870+
{
871+
Name: "v1alpha2",
872+
Served: true,
873+
Storage: false,
874+
},
875+
},
876+
Names: apiextensions.CustomResourceDefinitionNames{
877+
Plural: crdPlural,
878+
Singular: crdPlural,
879+
Kind: crdPlural,
880+
ListKind: "list" + crdPlural,
881+
},
882+
Scope: "Namespaced",
883+
},
884+
}
885+
886+
expectedCRDVersions := map[v1beta1.CustomResourceDefinitionVersion]struct{}{}
887+
for _, version := range mainCRD.Spec.Versions {
888+
key := v1beta1.CustomResourceDefinitionVersion{
889+
Name: version.Name,
890+
Served: version.Served,
891+
Storage: version.Storage,
892+
}
893+
expectedCRDVersions[key] = struct{}{}
894+
}
895+
896+
// Create the initial CSV
897+
cleanupCRD, err := createCRD(c, mainCRD)
898+
require.NoError(t, err)
899+
defer cleanupCRD()
900+
901+
mainCSV := newCSV(mainPackageStable, testNamespace, "", semver.MustParse("0.1.0"), nil, nil, mainNamedStrategy)
902+
903+
mainCatalogName := genName("mock-ocs-main-update2-")
780904

781-
// Update manifest
782-
updatedManifests := []registry.PackageManifest{
905+
// Create separate manifests for each CatalogSource
906+
mainManifests := []registry.PackageManifest{
783907
{
784908
PackageName: mainPackageName,
785909
Channels: []registry.PackageChannel{
786-
{Name: stableChannel, CurrentCSVName: updatedCSV.GetName()},
910+
{Name: stableChannel, CurrentCSVName: mainPackageStable},
787911
},
788912
DefaultChannelName: stableChannel,
789913
},
790914
}
791915

792-
updateInternalCatalog(t, c, crc, mainCatalogName, testNamespace, []apiextensions.CustomResourceDefinition{updatedCRD}, []v1alpha1.ClusterServiceVersion{mainCSV, updatedCSV}, updatedManifests)
916+
// Create the catalog sources
917+
_, cleanupMainCatalogSource := createInternalCatalogSource(t, c, crc, mainCatalogName, testNamespace, mainManifests, []apiextensions.CustomResourceDefinition{updatedCRD}, []v1alpha1.ClusterServiceVersion{mainCSV})
918+
defer cleanupMainCatalogSource()
919+
// Attempt to get the catalog source before creating install plan
920+
_, err = fetchCatalogSource(t, crc, mainCatalogName, testNamespace, catalogSourceRegistryPodSynced)
921+
require.NoError(t, err)
793922

794-
// Wait for subscription to update
795-
updatedSubscription, err := fetchSubscription(t, crc, testNamespace, subscriptionName, subscriptionHasCurrentCSV(updatedCSV.GetName()))
923+
subscriptionName := genName("sub-nginx-update2-")
924+
subscriptionCleanup := createSubscriptionForCatalog(t, crc, testNamespace, subscriptionName, mainCatalogName, mainPackageName, stableChannel, "", v1alpha1.ApprovalAutomatic)
925+
defer subscriptionCleanup()
926+
927+
subscription, err := fetchSubscription(t, crc, testNamespace, subscriptionName, subscriptionHasInstallPlanChecker)
796928
require.NoError(t, err)
929+
require.NotNil(t, subscription)
930+
require.NotNil(t, subscription.Status.InstallPlanRef)
931+
require.Equal(t, mainCSV.GetName(), subscription.Status.CurrentCSV)
797932

798-
// Verify installplan created and installed
799-
fetchedUpdatedInstallPlan, err := fetchInstallPlan(t, crc, updatedSubscription.Status.InstallPlanRef.Name, buildInstallPlanPhaseCheckFunc(v1alpha1.InstallPlanPhaseComplete))
933+
installPlanName := subscription.Status.InstallPlanRef.Name
934+
935+
// Wait for InstallPlan to be status: Complete before checking resource presence
936+
fetchedInstallPlan, err := fetchInstallPlan(t, crc, installPlanName, buildInstallPlanPhaseCheckFunc(v1alpha1.InstallPlanPhaseComplete))
800937
require.NoError(t, err)
801-
require.NotEqual(t, fetchedInstallPlan.GetName(), fetchedUpdatedInstallPlan.GetName())
802938

803-
// Wait for csv to update
804-
_, err = awaitCSV(t, crc, testNamespace, updatedCSV.GetName(), csvAnyChecker)
939+
require.Equal(t, v1alpha1.InstallPlanPhaseComplete, fetchedInstallPlan.Status.Phase)
940+
941+
// Fetch installplan again to check for unnecessary control loops
942+
fetchedInstallPlan, err = fetchInstallPlan(t, crc, fetchedInstallPlan.GetName(), func(fip *v1alpha1.InstallPlan) bool {
943+
compareResources(t, fetchedInstallPlan, fip)
944+
return true
945+
})
946+
require.NoError(t, err)
947+
948+
// Verify CSV is created
949+
_, err = awaitCSV(t, crc, testNamespace, mainCSV.GetName(), csvAnyChecker)
805950
require.NoError(t, err)
806951

807952
// Get the CRD to see if it is updated
808953
fetchedCRD, err := c.ApiextensionsV1beta1Interface().ApiextensionsV1beta1().CustomResourceDefinitions().Get(crdName, metav1.GetOptions{})
809954
require.NoError(t, err)
955+
require.Equal(t, len(fetchedCRD.Spec.Versions), len(mainCRD.Spec.Versions), "The CRD versions counts don't match")
810956

957+
fetchedCRDVersions := map[v1beta1.CustomResourceDefinitionVersion]struct{}{}
811958
for _, version := range fetchedCRD.Spec.Versions {
812959
key := v1beta1.CustomResourceDefinitionVersion{
813960
Name: version.Name,
814961
Served: version.Served,
815962
Storage: version.Storage,
816963
}
817-
_, ok := expectedCRDVersions[key]
818-
require.True(t, ok, "couldn't find %v in expected CRD versions: %#v", key, expectedCRDVersions)
964+
fetchedCRDVersions[key] = struct{}{}
965+
}
966+
967+
for _, version := range mainCRD.Spec.Versions {
968+
key := v1beta1.CustomResourceDefinitionVersion{
969+
Name: version.Name,
970+
Served: version.Served,
971+
Storage: version.Storage,
972+
}
973+
_, ok := fetchedCRDVersions[key]
974+
require.True(t, ok, "couldn't find %v in fetched CRD versions: %#v", key, fetchedCRDVersions)
819975
}
820976
})
821977
}

0 commit comments

Comments
 (0)