WARNING: THIS SITE IS A MIRROR OF GITHUB.COM / IT CANNOT LOGIN OR REGISTER ACCOUNTS / THE CONTENTS ARE PROVIDED AS-IS / THIS SITE ASSUMES NO RESPONSIBILITY FOR ANY DISPLAYED CONTENT OR LINKS / IF YOU FOUND SOMETHING MAY NOT GOOD FOR EVERYONE, CONTACT ADMIN AT ilovescratch@foxmail.com
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 59 additions & 5 deletions controllers/operatorpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,8 @@ func (r *OperatorPolicyReconciler) checkSubOverlap(
}

// applySubscriptionDefaults will set the subscription channel, source, and sourceNamespace when they are unset by
// utilizing the PackageManifest API.
// utilizing the PackageManifest API. If the PackageManifest can not be found, and a subscription already exists for the
// operator, then information from the found subscription will be used.
func (r *OperatorPolicyReconciler) applySubscriptionDefaults(
ctx context.Context, policy *policyv1beta1.OperatorPolicy, subscription *operatorv1alpha1.Subscription,
) error {
Expand All @@ -594,10 +595,14 @@ func (r *OperatorPolicyReconciler) applySubscriptionDefaults(
)
if err != nil {
if k8serrors.IsNotFound(err) {
return fmt.Errorf(
"%wthe subscription defaults could not be determined because the PackageManifest was not found",
ErrPackageManifest,
)
if !r.usingExistingSubIfFound(policy, subscription) {
Copy link
Contributor

@yiraeChristineKim yiraeChristineKim Oct 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have any status change for usingExistingSubIfFound? such as message

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't make a special status directly, it just allows the policy to be considered valid so that the status of the subscription can be updated (an invalid policy doesn't update the other conditions). The subscription status will have more information in some cases, for example explaining that the operator is not available in a catalog.

return fmt.Errorf(
"%wthe subscription defaults could not be determined because the PackageManifest was not found",
ErrPackageManifest,
)
}

return nil
}

log.Error(err, "Failed to get the PackageManifest", "name", subSpec.Package)
Expand Down Expand Up @@ -679,6 +684,55 @@ func (r *OperatorPolicyReconciler) applySubscriptionDefaults(
return nil
}

// usingExistingSubIfFound attempts to find an existing subscription for the operator, and uses values
// from that object to help build the desired subscription. It is meant to be used when the PackageManifest
// for the operator is not found. It returns true when a subscription was found and used successfully.
func (r *OperatorPolicyReconciler) usingExistingSubIfFound(
policy *policyv1beta1.OperatorPolicy, subscription *operatorv1alpha1.Subscription,
) bool {
if subscription.Namespace == "" {
// check for an already-known subscription to "adopt"
subs := policy.Status.RelatedObjsOfKind("Subscription")

if len(subs) == 1 {
// Note: RelatedObjsOfKind returns a map - in this case we just want the one object
for _, sub := range subs {
subscription.Namespace = sub.Object.Metadata.Namespace
}
}
}

if subscription.Namespace != "" {
watcher := opPolIdentifier(policy.Namespace, policy.Name)

gotSub, err := r.DynamicWatcher.Get(watcher, subscriptionGVK, subscription.Namespace, subscription.Name)
if err != nil || gotSub == nil {
return false
}

foundSub := new(operatorv1alpha1.Subscription)
if err := runtime.DefaultUnstructuredConverter.FromUnstructured(gotSub.Object, foundSub); err != nil {
return false
}

if subscription.Spec.Channel == "" {
subscription.Spec.Channel = foundSub.Spec.Channel
}

if subscription.Spec.CatalogSource == "" {
subscription.Spec.CatalogSource = foundSub.Spec.CatalogSource
}

if subscription.Spec.CatalogSourceNamespace == "" {
subscription.Spec.CatalogSourceNamespace = foundSub.Spec.CatalogSourceNamespace
}

return true
}

return false
}

// buildSubscription bootstraps the subscription spec defined in the operator policy
// with the apiversion and kind in preparation for resource creation.
// If an error is returned, it will include details on why the policy spec if invalid and
Expand Down
41 changes: 41 additions & 0 deletions test/e2e/case38_install_operator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,19 @@ var _ = Describe("Testing OperatorPolicy", Ordered, Label("supports-hosted"), fu
createObjWithParent(parentPolicyYAML, parentPolicyName,
opPolYAML, testNamespace, gvrPolicy, gvrOperatorPolicy)
})
AfterAll(func(ctx context.Context) {
By("Fixing the catalog source")
KubectlTarget("patch", "catalogsource", "operatorhubio-catalog", "--namespace=olm", "--type=json", "-p",
`[{"op": "replace", "path": "/spec/image", "value": "quay.io/operatorhubio/catalog:latest"}]`)

By("Waiting for a packagemanifest to reappear")
Eventually(func() error {
_, err := targetK8sDynamic.Resource(gvrPackageManifest).Namespace("default").Get(
ctx, "airflow-helm-operator", metav1.GetOptions{})

return err
}, olmWaitTimeout*2, 3).Should(Succeed())
})

It("Should create the Subscription with default values", func(ctx context.Context) {
By("Verifying the policy is compliant")
Expand Down Expand Up @@ -284,6 +297,34 @@ var _ = Describe("Testing OperatorPolicy", Ordered, Label("supports-hosted"), fu
sourceNamespace, _, _ := unstructured.NestedString(sub.Object, "spec", "sourceNamespace")
Expect(sourceNamespace).To(Equal("olm"))
})

It("Should adopt an existing subscription if removed from the catalog", func(ctx context.Context) {
By("Breaking the catalog source")
KubectlTarget("patch", "catalogsource", "operatorhubio-catalog", "--namespace=olm", "--type=json", "-p",
`[{"op": "replace", "path": "/spec/image", "value": "quay.io/operatorhubio/catalog:broken"}]`)

By("ensuring the packagemanifest is gone")
Eventually(func() error {
_, err := targetK8sDynamic.Resource(gvrPackageManifest).Namespace("default").Get(
ctx, "airflow-helm-operator", metav1.GetOptions{})

return err
}, olmWaitTimeout, 3).ShouldNot(Succeed())

By("Checking the validation condition")
check(
opPolName,
false,
[]policyv1.RelatedObject{},
metav1.Condition{
Type: "ValidPolicySpec",
Status: metav1.ConditionTrue,
Reason: "PolicyValidated",
Message: `the policy spec is valid`,
},
`the policy spec is valid`,
)
})
})

Describe("Testing an operator policy with invalid partial defaults", Ordered, func() {
Expand Down
6 changes: 6 additions & 0 deletions test/e2e/e2e_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ var (
gvrOperatorGroup schema.GroupVersionResource
gvrInstallPlan schema.GroupVersionResource
gvrClusterServiceVersion schema.GroupVersionResource
gvrPackageManifest schema.GroupVersionResource
defaultImageRegistry string
IsHosted bool
targetK8sClient kubernetes.Interface
Expand Down Expand Up @@ -122,6 +123,11 @@ var _ = BeforeSuite(func() {
Version: "v1alpha1",
Resource: "clusterserviceversions",
}
gvrPackageManifest = schema.GroupVersionResource{
Group: "packages.operators.coreos.com",
Version: "v1",
Resource: "packagemanifests",
}
gvrAPIService = schema.GroupVersionResource{
Group: "apiregistration.k8s.io",
Version: "v1",
Expand Down