Skip to content

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.

jmprusi added 2 commits May 16, 2023 10:52
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>
@jmprusi jmprusi force-pushed the 202-resolvedBundleSource branch from f28c545 to c6a638a Compare May 16, 2023 08:53
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