Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds new Status condition "Resolved" and related status field "resolvedBundleResource" to Operator CR #213

Merged
merged 2 commits into from
May 16, 2023

Conversation

jmprusi
Copy link
Member

@jmprusi jmprusi commented May 15, 2023

Description

This PR extends the status conditions of the Operator CR and adds a new status field.

  • New condition "Resolved" exposes the current status of resolving the desired operator image.
  • New status field "resolvedBundleResource" contains the resolved operator image in case it was successful.

Fixes #202

Reviewer Checklist

  • API Go Documentation
  • Tests: Unit Tests (and E2E Tests, if appropriate)
  • Comprehensive Commit Messages
  • Links to related GitHub Issue(s)

@jmprusi jmprusi force-pushed the 202-resolvedBundleSource branch 2 times, most recently from 50dd855 to f28c545 Compare May 15, 2023 15:05
Copy link
Contributor

@tmshort tmshort left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me.
But tests are failing.


ReasonInstallationSucceeded = "InstallationSucceeded"
ReasonResolutionFailed = "ResolutionFailed"
ReasonResolutionUnknown = "ResolutionUnknown"
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: these ought to be alphabetical, but others are not. :(

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed.

This commits adds a new status conditions: "Resolved" that represent if
the Operator-Controller was able to resolv the desired operator image or
not.

Also, extends the status with a new field: "resolvedBundleResource" that
contains the resolved operator image.

Signed-off-by: Joaquim Moreno Prusi <joaquim@redhat.com>
Signed-off-by: Joaquim Moreno Prusi <joaquim@redhat.com>
Copy link
Member

@m1kola m1kola left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 16, 2023
@tmshort
Copy link
Contributor

tmshort commented May 16, 2023

Still looks good to me. Merging since it's kinda-sorta blocking #201

@tmshort tmshort merged commit 4ea83e6 into operator-framework:main May 16, 2023
@jmprusi jmprusi deleted the 202-resolvedBundleSource branch May 18, 2023 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add status.resolvedBundleSource
3 participants