From eee462241711372d35c2670fd67ba720cbd19b4e Mon Sep 17 00:00:00 2001 From: Evan Cordell Date: Thu, 24 May 2018 17:40:41 -0400 Subject: [PATCH 1/3] fix(e2e): make e2e local scripts work again --- Makefile | 4 ++-- scripts/run_e2e_docker.sh | 2 +- scripts/run_e2e_local.sh | 2 +- test/e2e/e2e-values-shift.yaml | 6 ++++++ test/e2e/e2e-values.yaml | 5 +++++ 5 files changed, 15 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 664314ae956..4eb85a349d8 100644 --- a/Makefile +++ b/Makefile @@ -155,10 +155,10 @@ generate-mock-client: $(counterfeiter) make gen-all: gen-ci codegen generate-mock-client # make ver=0.3.0 tectonic-release -make tectonic-release: +tectonic-release: ./scripts/package-release.sh $(ver) deploy/tectonic-alm-operator/manifests/$(ver) deploy/tectonic-alm-operator/values.yaml # make ver=0.3.0 release -make release: +release: ./scripts/package-release.sh $(ver) deploy/upstream/manifests/$(ver) deploy/upstream/values.yaml diff --git a/scripts/run_e2e_docker.sh b/scripts/run_e2e_docker.sh index ad36bff8934..803e0e9ee27 100755 --- a/scripts/run_e2e_docker.sh +++ b/scripts/run_e2e_docker.sh @@ -14,7 +14,7 @@ echo "namespace: ${namespace}" >> ${tmpdir}/e2e-values.yaml echo "watchedNamespaces: ${namespace}" >> ${tmpdir}/e2e-values.yaml echo "catalog_namespace: ${namespace}" >> ${tmpdir}/e2e-values.yaml -./scripts/package-release.sh ver=1.0.0-e2e test/e2e/resources ${tmpdir}/e2e-values.yaml +./scripts/package-release.sh 1.0.0-e2e test/e2e/resources ${tmpdir}/e2e-values.yaml function cleanup { kubectl delete namespace ${namespace} diff --git a/scripts/run_e2e_local.sh b/scripts/run_e2e_local.sh index 47bcd0637ce..318f113fd6a 100755 --- a/scripts/run_e2e_local.sh +++ b/scripts/run_e2e_local.sh @@ -15,7 +15,7 @@ echo "namespace: ${namespace}" >> ${tmpdir}/e2e-values.yaml echo "watchedNamespaces: ${namespace}" >> ${tmpdir}/e2e-values.yaml echo "catalog_namespace: ${namespace}" >> ${tmpdir}/e2e-values.yaml -./scripts/package-release.sh ver=1.0.0-e2e test/e2e/resources ${tmpdir}/e2e-values.yaml +./scripts/package-release.sh 1.0.0-e2e test/e2e/resources ${tmpdir}/e2e-values.yaml function cleanup { kubectl delete namespace ${namespace} diff --git a/test/e2e/e2e-values-shift.yaml b/test/e2e/e2e-values-shift.yaml index 28c87000afa..cb9c1c8a6c2 100644 --- a/test/e2e/e2e-values-shift.yaml +++ b/test/e2e/e2e-values-shift.yaml @@ -31,3 +31,9 @@ e2e: ref: quay.io/coreos/olm-e2e:local job_name: e2e + + +catalog_sources: + - tectonic-ocs + - tectonic-components + - upstream-components diff --git a/test/e2e/e2e-values.yaml b/test/e2e/e2e-values.yaml index 58f86429ee7..1802c4d6b82 100644 --- a/test/e2e/e2e-values.yaml +++ b/test/e2e/e2e-values.yaml @@ -21,3 +21,8 @@ e2e: ref: quay.io/coreos/olm-e2e:local job_name: e2e + +catalog_sources: + - tectonic-ocs + - tectonic-components + - upstream-components From 48a8b1fd926df94f810a8e924af7e1c7143a8cec Mon Sep 17 00:00:00 2001 From: Evan Cordell Date: Fri, 25 May 2018 16:28:57 -0400 Subject: [PATCH 2/3] fix(subscriptions): directly include apiversion/kind of created resource k8s resources only have TypeMeta when serialized, so pull in the apiversion and kind of the resolved installplan of the type we created --- .../operators/catalog/subscriptions.go | 4 +- .../operators/catalog/subscriptions_test.go | 40 ++++++++++++++----- 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/pkg/controller/operators/catalog/subscriptions.go b/pkg/controller/operators/catalog/subscriptions.go index 1f9bc083f59..9d9389ee48c 100644 --- a/pkg/controller/operators/catalog/subscriptions.go +++ b/pkg/controller/operators/catalog/subscriptions.go @@ -108,8 +108,8 @@ func (o *Operator) syncSubscription(sub *v1alpha1.Subscription) (*v1alpha1.Subsc sub.Status.Install = &v1alpha1.InstallPlanReference{ UID: res.GetUID(), Name: res.GetName(), - APIVersion: res.APIVersion, - Kind: res.Kind, + APIVersion: ipv1alpha1.SchemeGroupVersion.String(), + Kind: ipv1alpha1.InstallPlanKind, } return sub, nil } diff --git a/pkg/controller/operators/catalog/subscriptions_test.go b/pkg/controller/operators/catalog/subscriptions_test.go index 1f1cbfb2521..5d4ab252d8c 100644 --- a/pkg/controller/operators/catalog/subscriptions_test.go +++ b/pkg/controller/operators/catalog/subscriptions_test.go @@ -150,7 +150,11 @@ func TestSyncSubscription(t *testing.T) { CurrentCSV: "latest-and-greatest", LastUpdated: earliestTime, State: v1alpha1.SubscriptionStateUpgradePending, - Install: &v1alpha1.InstallPlanReference{Name: "existing-install"}, + Install: &v1alpha1.InstallPlanReference{ + Kind: ipv1alpha1.InstallPlanKind, + APIVersion: ipv1alpha1.SchemeGroupVersion.String(), + Name: "existing-install", + }, }, }}, expected: expected{ @@ -168,7 +172,11 @@ func TestSyncSubscription(t *testing.T) { CurrentCSV: "latest-and-greatest", LastUpdated: earliestTime, State: v1alpha1.SubscriptionStateUpgradePending, - Install: &v1alpha1.InstallPlanReference{Name: "existing-install"}, + Install: &v1alpha1.InstallPlanReference{ + Kind: ipv1alpha1.InstallPlanKind, + APIVersion: ipv1alpha1.SchemeGroupVersion.String(), + Name: "existing-install", + }, }, }, err: "", @@ -333,7 +341,11 @@ func TestSyncSubscription(t *testing.T) { }, Status: v1alpha1.SubscriptionStatus{ CurrentCSV: "pending", - Install: &v1alpha1.InstallPlanReference{Name: "existing-install"}, + Install: &v1alpha1.InstallPlanReference{ + Kind: ipv1alpha1.InstallPlanKind, + APIVersion: ipv1alpha1.SchemeGroupVersion.String(), + Name: "existing-install", + }, }, }}, expected: expected{ @@ -372,7 +384,9 @@ func TestSyncSubscription(t *testing.T) { Status: v1alpha1.SubscriptionStatus{ CurrentCSV: "latest-and-greatest", Install: &v1alpha1.InstallPlanReference{ - Name: "dead-install", + Kind: ipv1alpha1.InstallPlanKind, + APIVersion: ipv1alpha1.SchemeGroupVersion.String(), + Name: "dead-install", }, }, }}, @@ -413,8 +427,10 @@ func TestSyncSubscription(t *testing.T) { Status: v1alpha1.SubscriptionStatus{ CurrentCSV: "latest-and-greatest", Install: &v1alpha1.InstallPlanReference{ - UID: types.UID("UID-OK"), - Name: "installplan-1", + Kind: ipv1alpha1.InstallPlanKind, + APIVersion: ipv1alpha1.SchemeGroupVersion.String(), + UID: types.UID("UID-OK"), + Name: "installplan-1", }, State: v1alpha1.SubscriptionStateUpgradePending, }, @@ -486,8 +502,10 @@ func TestSyncSubscription(t *testing.T) { Status: v1alpha1.SubscriptionStatus{ CurrentCSV: "latest-and-greatest", Install: &v1alpha1.InstallPlanReference{ - UID: types.UID("UID-OK"), - Name: "installplan-1", + Kind: ipv1alpha1.InstallPlanKind, + APIVersion: ipv1alpha1.SchemeGroupVersion.String(), + UID: types.UID("UID-OK"), + Name: "installplan-1", }, State: v1alpha1.SubscriptionStateUpgradePending, }, @@ -563,8 +581,10 @@ func TestSyncSubscription(t *testing.T) { Status: v1alpha1.SubscriptionStatus{ CurrentCSV: "latest-and-greatest", Install: &v1alpha1.InstallPlanReference{ - UID: types.UID("UID-OK"), - Name: "installplan-1", + Kind: ipv1alpha1.InstallPlanKind, + APIVersion: ipv1alpha1.SchemeGroupVersion.String(), + UID: types.UID("UID-OK"), + Name: "installplan-1", }, State: v1alpha1.SubscriptionStateUpgradePending, }, From 1bad32f6010c3bb3216b47396e6726bb04d95ec8 Mon Sep 17 00:00:00 2001 From: Evan Cordell Date: Tue, 29 May 2018 10:59:02 -0400 Subject: [PATCH 3/3] feat(ownerutil): update ownerutil to infer GVK for OLM types --- .../operators/catalog/subscriptions.go | 11 +--- .../operators/catalog/subscriptions_test.go | 53 +++++++++++----- pkg/lib/ownerutil/util.go | 62 ++++++++++++++----- 3 files changed, 87 insertions(+), 39 deletions(-) diff --git a/pkg/controller/operators/catalog/subscriptions.go b/pkg/controller/operators/catalog/subscriptions.go index 9d9389ee48c..6549758513f 100644 --- a/pkg/controller/operators/catalog/subscriptions.go +++ b/pkg/controller/operators/catalog/subscriptions.go @@ -7,6 +7,7 @@ import ( ipv1alpha1 "github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/installplan/v1alpha1" "github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/subscription/v1alpha1" + "github.com/operator-framework/operator-lifecycle-manager/pkg/lib/ownerutil" log "github.com/sirupsen/logrus" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -87,15 +88,7 @@ func (o *Operator) syncSubscription(sub *v1alpha1.Subscription) (*v1alpha1.Subsc Approval: sub.GetInstallPlanApproval(), }, } - owner := []metav1.OwnerReference{ - { - APIVersion: v1alpha1.SchemeGroupVersion.String(), - Kind: v1alpha1.SubscriptionKind, - Name: sub.GetName(), - UID: sub.GetUID(), - }, - } - ip.SetOwnerReferences(owner) + ownerutil.AddNonBlockingOwner(ip, sub) ip.SetGenerateName(fmt.Sprintf("install-%s-", sub.Status.CurrentCSV)) ip.SetNamespace(sub.GetNamespace()) res, err := o.client.InstallplanV1alpha1().InstallPlans(sub.GetNamespace()).Create(ip) diff --git a/pkg/controller/operators/catalog/subscriptions_test.go b/pkg/controller/operators/catalog/subscriptions_test.go index 5d4ab252d8c..a8d36c4bb31 100644 --- a/pkg/controller/operators/catalog/subscriptions_test.go +++ b/pkg/controller/operators/catalog/subscriptions_test.go @@ -20,12 +20,25 @@ import ( "github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/clientset/versioned/fake" "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry" "github.com/operator-framework/operator-lifecycle-manager/pkg/fakes" + "k8s.io/apimachinery/pkg/util/diff" +) + +var ( + blockOwnerDeletion = false + isController = false ) func RequireActions(t *testing.T, expected, actual []core.Action) { require.EqualValues(t, len(expected), len(actual), "Expected\n\t%#v\ngot\n\t%#v", expected, actual) for i, a := range actual { e := expected[i] + switch c := e.(type) { + case core.CreateActionImpl: + ac := a.(core.CreateActionImpl) + cObj := c.Object + acObj := ac.Object + require.True(t, equality.Semantic.DeepEqual(cObj, acObj), "Expected\n\t%#v\ngot\n\t%#v\n\tdiff:%s", cObj, acObj, diff.ObjectDiff(cObj, acObj)) + } require.True(t, equality.Semantic.DeepEqual(e, a), "Expected\n\t%#v\ngot\n\t%#v", e, a) } } @@ -400,10 +413,12 @@ func TestSyncSubscription(t *testing.T) { Namespace: "fairy-land", OwnerReferences: []metav1.OwnerReference{ { - APIVersion: "app.coreos.com/v1alpha1", - Kind: "Subscription-v1", - Name: "test-subscription", - UID: types.UID("subscription-uid"), + APIVersion: "app.coreos.com/v1alpha1", + Kind: "Subscription-v1", + Name: "test-subscription", + UID: types.UID("subscription-uid"), + BlockOwnerDeletion: &blockOwnerDeletion, + Controller: &isController, }, }, }, @@ -475,10 +490,12 @@ func TestSyncSubscription(t *testing.T) { Namespace: "fairy-land", OwnerReferences: []metav1.OwnerReference{ { - APIVersion: "app.coreos.com/v1alpha1", - Kind: "Subscription-v1", - Name: "test-subscription", - UID: types.UID("subscription-uid"), + APIVersion: "app.coreos.com/v1alpha1", + Kind: "Subscription-v1", + Name: "test-subscription", + UID: types.UID("subscription-uid"), + BlockOwnerDeletion: &blockOwnerDeletion, + Controller: &isController, }, }, }, @@ -553,10 +570,12 @@ func TestSyncSubscription(t *testing.T) { Namespace: "fairy-land", OwnerReferences: []metav1.OwnerReference{ { - APIVersion: "app.coreos.com/v1alpha1", - Kind: "Subscription-v1", - Name: "test-subscription", - UID: types.UID("subscription-uid"), + APIVersion: "app.coreos.com/v1alpha1", + Kind: "Subscription-v1", + Name: "test-subscription", + UID: types.UID("subscription-uid"), + BlockOwnerDeletion: &blockOwnerDeletion, + Controller: &isController, }, }, }, @@ -628,10 +647,12 @@ func TestSyncSubscription(t *testing.T) { Namespace: "fairy-land", OwnerReferences: []metav1.OwnerReference{ { - APIVersion: "app.coreos.com/v1alpha1", - Kind: "Subscription-v1", - Name: "test-subscription", - UID: types.UID("subscription-uid"), + APIVersion: "app.coreos.com/v1alpha1", + Kind: "Subscription-v1", + Name: "test-subscription", + UID: types.UID("subscription-uid"), + BlockOwnerDeletion: &blockOwnerDeletion, + Controller: &isController, }, }, }, diff --git a/pkg/lib/ownerutil/util.go b/pkg/lib/ownerutil/util.go index 55c1deee743..08bb10bfbcb 100644 --- a/pkg/lib/ownerutil/util.go +++ b/pkg/lib/ownerutil/util.go @@ -2,7 +2,11 @@ package ownerutil import ( "github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis" - "github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/clusterserviceversion/v1alpha1" + csv1alpha1 "github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/catalogsource/v1alpha1" + csvv1alpha1 "github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/clusterserviceversion/v1alpha1" + ipv1alpha1 "github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/installplan/v1alpha1" + subv1alpha1 "github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/subscription/v1alpha1" + log "github.com/sirupsen/logrus" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" ) @@ -22,20 +26,10 @@ func IsOwnedBy(object metav1.Object, owner Owner) bool { return false } +// AddNonBlockingOwner adds a nonblocking owner to the ownerref list. func AddNonBlockingOwner(object metav1.Object, owner Owner) { - // TODO: Remove as soon as possible - // This is a hack, for some reason CSVs that we get out of the informer are missing - // TypeMeta, which means we can't get the APIVersion or Kind generically here. - // The underlying issue should be found and fixes as soon as possible - // This needs to be removed before a new APIVersion is cut - if _, ok := owner.(*v1alpha1.ClusterServiceVersion); ok { - owner.SetGroupVersionKind(schema.GroupVersionKind{ - Group: apis.GroupName, - Version: v1alpha1.GroupVersion, - Kind: v1alpha1.ClusterServiceVersionKind, - }) - } - + // Most of the time we won't have TypeMeta on the object, so we infer it for types we know about + inferGroupVersionKind(owner) blockOwnerDeletion := false isController := false @@ -55,3 +49,43 @@ func AddNonBlockingOwner(object metav1.Object, owner Owner) { }) object.SetOwnerReferences(ownerRefs) } + +// inferGroupVersionKind adds TypeMeta to an owner so that it can be written to an ownerref. +// TypeMeta is generally only known at serialization time, so we often won't know what GVK an owner has. +// For the types we know about, we can add the GVK of the apis that we're using the interact with the object. +func inferGroupVersionKind(owner Owner) { + if !owner.GroupVersionKind().Empty() { + // owner already has TypeMeta, no inference needed + return + } + + switch v := owner.(type) { + case *csvv1alpha1.ClusterServiceVersion: + owner.SetGroupVersionKind(schema.GroupVersionKind{ + Group: apis.GroupName, + Version: csvv1alpha1.GroupVersion, + Kind: csvv1alpha1.ClusterServiceVersionKind, + }) + case *ipv1alpha1.InstallPlan: + owner.SetGroupVersionKind(schema.GroupVersionKind{ + Group: apis.GroupName, + Version: ipv1alpha1.GroupVersion, + Kind: ipv1alpha1.InstallPlanKind, + }) + case *subv1alpha1.Subscription: + owner.SetGroupVersionKind(schema.GroupVersionKind{ + Group: apis.GroupName, + Version: subv1alpha1.GroupVersion, + Kind: subv1alpha1.SubscriptionKind, + }) + case *csv1alpha1.CatalogSource: + owner.SetGroupVersionKind(schema.GroupVersionKind{ + Group: apis.GroupName, + Version: csv1alpha1.GroupVersion, + Kind: csv1alpha1.CatalogSourceKind, + }) + default: + log.Warnf("could not infer GVK for object: %#v, %#v", v, owner) + } + return +}