add OperatorGroup reconciliation #2517

akihikokuroda · 2021-12-10T17:48:08Z

Signed-off-by: akihikokuroda [email protected]

Description of the change:
Add OperatorGroup reconciliation in Catalog operator so that the operator group creation can trigger InstallPlan sync.

Motivation for the change:
Closes #2516
Reviewer Checklist

Implementation matches the proposed design, or proposal is updated to match implementation
Sufficient unit test coverage
Sufficient end-to-end test coverage
Docs updated or added to /doc
Commit messages sensible and descriptive

openshift-ci · 2021-12-10T17:48:12Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: akihikokuroda
To complete the pull request process, please assign kevinrizza after the PR has been reviewed.
You can assign the PR to them by writing /assign @kevinrizza in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci · 2021-12-10T17:48:18Z

Hi @akihikokuroda. Thanks for your PR.

I'm waiting for a operator-framework member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

perdasilva · 2021-12-13T17:50:12Z

Hey @akihikokuroda thanks for this! Would it be possible to add unit tests at all?

akihikokuroda · 2021-12-13T18:00:07Z

@perdasilva Thanks for comment. I'll look into what unit test needs for this change.

akihikokuroda · 2021-12-14T17:46:57Z

I added a small unit test for operatorgroup sync.

awgreene · 2022-01-14T20:50:46Z

pkg/controller/operators/catalog/operator.go

 	return o.triggerInstallPlanRetry(obj)
 }

+func (o *Operator) syncOperatorGroup(obj interface{}) (syncError error) {
+	// Assert as metav1.Object


Nit - I don't think this comment adds any value.

Thanks! I'll take it out.

awgreene · 2022-01-14T20:53:53Z

pkg/controller/operators/catalog/installplan_sync.go

+		out := ip.DeepCopy()
+		out.Status.Phase = v1alpha1.InstallPlanPhaseInstalling
+		now := o.now()
+		out.Status.SetCondition(v1alpha1.ConditionFailed(v1alpha1.InstallPlanInstalled, v1alpha1.InstallPlanReasonComponentFailed, "OperatorGroup updated", &now))


Is this the right condition to add? If we are re-triggering the installPlan I don't think a component failed anymore.

I see. That's right. I'll change to ConditionMet. Thanks!

akihikokuroda · 2022-01-18T20:29:25Z

/hold

akihikokuroda · 2022-01-18T21:02:30Z

I saw this panic. It's probably unrelated the changes in this PR so far but I add some code to prevent this in this PR. This panic causes the same e2e failure.

2022-01-18T18:23:06.480373869Z stderr F time="2022-01-18T18:23:06Z" level=debug msg="Got source event: grpc.SourceState{Key:registry.CatalogKey{Name:\"mock-ocs-main-m48zw\", Namespace:\"operator-lifecycle-manager\"}, State:2}"
2022-01-18T18:23:06.480396869Z stderr F time="2022-01-18T18:23:06Z" level=info msg="state.Key.Namespace=operator-lifecycle-manager state.Key.Name=mock-ocs-main-m48zw state.State=READY"
2022-01-18T18:23:06.490549584Z stderr F time="2022-01-18T18:23:06Z" level=error msg="error updating InstallPlan status" id=ilvfL ip=ip namespace=ns-d8pd9 phase=Installing updateError="Operation cannot be fulfilled on installplans.operators.coreos.com \"ip\": the object h\
as been modified; please apply your changes to the latest version and try again"
2022-01-18T18:23:06.495193536Z stderr F panic: runtime error: invalid memory address or nil pointer dereference
2022-01-18T18:23:06.495203736Z stderr F [signal SIGSEGV: segmentation violation code=0x1 addr=0xf4 pc=0x942671d]
2022-01-18T18:23:06.495206836Z stderr F
2022-01-18T18:23:06.495211136Z stderr F goroutine 433 [running]:
2022-01-18T18:23:06.495315737Z stderr F github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/catalog.(*Operator).syncInstallPlans(0xbeae410, {0x98647e0, 0xc412120})
2022-01-18T18:23:06.495387838Z stderr F         /home/runner/work/operator-lifecycle-manager/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go:1593 +0x9ed
2022-01-18T18:23:06.495486339Z stderr F github.com/operator-framework/operator-lifecycle-manager/pkg/lib/queueinformer.LegacySyncHandler.ToSyncerWithDelete.func1({0x9b5e758, 0xbc7a150}, {0x9b55b6c, 0xbfdb240})
2022-01-18T18:23:06.495491739Z stderr F         /home/runner/work/operator-lifecycle-manager/operator-lifecycle-manager/pkg/lib/queueinformer/queueinformer.go:181 +0xed
2022-01-18T18:23:06.495630841Z stderr F github.com/operator-framework/operator-lifecycle-manager/pkg/lib/kubestate.SyncFunc.Sync(0xbc9e7f0, {0x9b5e758, 0xbc7a150}, {0x9b55b6c, 0xbfdb240})
2022-01-18T18:23:06.495714042Z stderr F         /home/runner/work/operator-lifecycle-manager/operator-lifecycle-manager/pkg/lib/kubestate/kubestate.go:184 +0x3c
2022-01-18T18:23:06.495779942Z stderr F github.com/operator-framework/operator-lifecycle-manager/pkg/lib/queueinformer.(*QueueInformer).Sync(...)
2022-01-18T18:23:06.495784942Z stderr F         /home/runner/work/operator-lifecycle-manager/operator-lifecycle-manager/pkg/lib/queueinformer/queueinformer.go:35
2022-01-18T18:23:06.495876443Z stderr F github.com/operator-framework/operator-lifecycle-manager/pkg/lib/queueinformer.(*operator).processNextWorkItem(0xbe6ad90, {0x9b5e758, 0xbc7a150}, 0xbca79e0)
2022-01-18T18:23:06.495919944Z stderr F         /home/runner/work/operator-lifecycle-manager/operator-lifecycle-manager/pkg/lib/queueinformer/queueinformer_operator.go:287 +0x887
2022-01-18T18:23:06.496032045Z stderr F github.com/operator-framework/operator-lifecycle-manager/pkg/lib/queueinformer.(*operator).worker(0xbe6ad90, {0x9b5e758, 0xbc7a150}, 0xbca79e0)
2022-01-18T18:23:06.496135646Z stderr F         /home/runner/work/operator-lifecycle-manager/operator-lifecycle-manager/pkg/lib/queueinformer/queueinformer_operator.go:231 +0x39
2022-01-18T18:23:06.496140746Z stderr F created by github.com/operator-framework/operator-lifecycle-manager/pkg/lib/queueinformer.(*operator).start
2022-01-18T18:23:06.496233648Z stderr F         /home/runner/work/operator-lifecycle-manager/operator-lifecycle-manager/pkg/lib/queueinformer/queueinformer_operator.go:221 +0x560

akihikokuroda · 2022-01-18T21:33:04Z

/unhold

Signed-off-by: akihikokuroda <[email protected]>

akihikokuroda · 2022-02-21T15:48:46Z

/hold

openshift-merge-robot · 2023-02-21T14:29:55Z

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

perdasilva · 2023-02-28T13:04:04Z

@akihikokuroda could you please resolve conflicts?

perdasilva · 2024-02-19T13:33:24Z

closing PR as stale. Please re-open if it's still important.

openshift-ci bot requested review from benluddy and ecordell December 10, 2021 17:48

openshift-ci bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Dec 10, 2021

akihikokuroda force-pushed the ogsync branch 3 times, most recently from 048f8b0 to 5cce51b Compare December 12, 2021 19:05

timflannagan removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Dec 12, 2021

akihikokuroda mentioned this pull request Dec 13, 2021

Add system constraint provider logic to resolver #2523

Merged

5 tasks

akihikokuroda force-pushed the ogsync branch from 5cce51b to fd3c0d1 Compare December 13, 2021 13:50

akihikokuroda force-pushed the ogsync branch 2 times, most recently from a4dfccb to 4819199 Compare December 14, 2021 17:46

akihikokuroda force-pushed the ogsync branch from 4819199 to 2ad3630 Compare January 11, 2022 19:32

akihikokuroda mentioned this pull request Jan 11, 2022

REQUEST: New membership for akihikokuroda operator-framework/community#58

Closed

6 tasks

akihikokuroda force-pushed the ogsync branch 2 times, most recently from b94df46 to eb082c9 Compare January 13, 2022 13:11

awgreene reviewed Jan 14, 2022

View reviewed changes

akihikokuroda force-pushed the ogsync branch from eb082c9 to d3876de Compare January 15, 2022 02:15

akihikokuroda requested a review from awgreene January 15, 2022 02:15

akihikokuroda force-pushed the ogsync branch from d3876de to 5081254 Compare January 15, 2022 02:43

timflannagan added the runtime label Jan 18, 2022

akihikokuroda force-pushed the ogsync branch 2 times, most recently from 9c4627a to 4dcb48b Compare January 18, 2022 17:49

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 18, 2022

akihikokuroda force-pushed the ogsync branch 2 times, most recently from 0409a45 to 00be3ce Compare January 18, 2022 21:31

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 18, 2022

akihikokuroda force-pushed the ogsync branch 2 times, most recently from b6213fa to 7d7f77d Compare January 28, 2022 18:25

akihikokuroda force-pushed the ogsync branch 2 times, most recently from e25638f to ef0bfd8 Compare February 4, 2022 22:31

akihikokuroda force-pushed the ogsync branch 2 times, most recently from b050ace to e13258f Compare February 20, 2022 15:47

add OperatorGroup reconciliation

7552fa8

Signed-off-by: akihikokuroda <[email protected]>

akihikokuroda force-pushed the ogsync branch from e13258f to 7552fa8 Compare February 20, 2022 23:44

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 21, 2022

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 21, 2023

perdasilva closed this Feb 19, 2024

add OperatorGroup reconciliation #2517

add OperatorGroup reconciliation #2517

Uh oh!

Conversation

akihikokuroda commented Dec 10, 2021

Uh oh!

openshift-ci bot commented Dec 10, 2021

Uh oh!

openshift-ci bot commented Dec 10, 2021

Uh oh!

perdasilva commented Dec 13, 2021

Uh oh!

akihikokuroda commented Dec 13, 2021

Uh oh!

akihikokuroda commented Dec 14, 2021

Uh oh!

awgreene Jan 14, 2022

Choose a reason for hiding this comment

Uh oh!

akihikokuroda Jan 15, 2022

Choose a reason for hiding this comment

Uh oh!

awgreene Jan 14, 2022

Choose a reason for hiding this comment

Uh oh!

akihikokuroda Jan 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akihikokuroda commented Jan 18, 2022

Uh oh!

akihikokuroda commented Jan 18, 2022

Uh oh!

akihikokuroda commented Jan 18, 2022

Uh oh!

akihikokuroda commented Feb 21, 2022

Uh oh!

openshift-merge-robot commented Feb 21, 2023

Uh oh!

perdasilva commented Feb 28, 2023

Uh oh!

perdasilva commented Feb 19, 2024

Uh oh!

Uh oh!

akihikokuroda Jan 15, 2022 •

edited

Loading