operator-lifecycle-manager icon indicating copy to clipboard operation
operator-lifecycle-manager copied to clipboard

add OperatorGroup reconciliation

Open akihikokuroda opened this issue 4 years ago • 9 comments

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

akihikokuroda avatar Dec 10 '21 17:12 akihikokuroda

[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:

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

openshift-ci[bot] avatar Dec 10 '21 17:12 openshift-ci[bot]

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.

openshift-ci[bot] avatar Dec 10 '21 17:12 openshift-ci[bot]

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

perdasilva avatar Dec 13 '21 17:12 perdasilva

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

akihikokuroda avatar Dec 13 '21 18:12 akihikokuroda

I added a small unit test for operatorgroup sync.

akihikokuroda avatar Dec 14 '21 17:12 akihikokuroda

/hold

akihikokuroda avatar Jan 18 '22 20:01 akihikokuroda

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 avatar Jan 18 '22 21:01 akihikokuroda

/unhold

akihikokuroda avatar Jan 18 '22 21:01 akihikokuroda

/hold

akihikokuroda avatar Feb 21 '22 15:02 akihikokuroda

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.

openshift-merge-robot avatar Feb 21 '23 14:02 openshift-merge-robot

@akihikokuroda could you please resolve conflicts?

perdasilva avatar Feb 28 '23 13:02 perdasilva

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

perdasilva avatar Feb 19 '24 13:02 perdasilva