Skip to content
This repository has been archived by the owner on Jul 30, 2021. It is now read-only.

Update capability and name for the Crunchy PostgreSQL Operator. #196

Merged
merged 1 commit into from Apr 9, 2019
Merged

Update capability and name for the Crunchy PostgreSQL Operator. #196

merged 1 commit into from Apr 9, 2019

Conversation

jkatz
Copy link
Contributor

@jkatz jkatz commented Mar 20, 2019

No description provided.

@openshift-ci-robot openshift-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 20, 2019
@openshift-ci-robot
Copy link
Collaborator

Hi @jkatz. Thanks for your PR.

I'm waiting for a operator-framework or openshift 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-robot openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 20, 2019
@dmesser
Copy link
Collaborator

dmesser commented Mar 20, 2019

Thanks for this update. Would you also be able to add a couple of examples for the CRs in alm-examples as well as some links to documentation?

@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 20, 2019
@jkatz
Copy link
Contributor Author

jkatz commented Mar 20, 2019

Hi Daniel,

I have made the suggested updates and a few more. We don't have any alm-examples just yet, but I can work with the team to try to have something for the upcoming 4.0 release.

Thanks!

Jonathan

@jomkz
Copy link
Contributor

jomkz commented Mar 20, 2019

Correct me if I'm wrong, but does the channel name field in postgresql.package.yaml need to be updated as well to correspond with the maturity value set to stable now in the CSV?

@jkatz
Copy link
Contributor Author

jkatz commented Mar 20, 2019

Hi John,

You are correct, I missed that! I have updated the commit to reflect that.

Thanks!

Jonathan

@robszumski
Copy link
Collaborator

Note that bumping the maturity is due to the Operator supporting provision, deletion, backup/restore of Postgres. This has always been the case, but we were under reporting the maturity of this Operator.

@SamiSousa
Copy link
Contributor

@jkatz Could you push a 'dummy' PR? We are currently testing our CI for community-operators, and the only way we know of updating the build is to add changes to the PR. Apologies for the inconvenience.

@jkatz
Copy link
Contributor Author

jkatz commented Mar 20, 2019

@SamiSousa Pushed.

@jkatz
Copy link
Contributor Author

jkatz commented Mar 20, 2019

Per @robszumski Also wanted to note that the operator does support scaling up/down PostgreSQL instances (which means adding/removing replicas), and failover (manual + automatic) too 😄

@jkatz
Copy link
Contributor Author

jkatz commented Mar 26, 2019

Hi! Is there anything else we need to do on our end for this patch? I believe I have incorporated all the requested feedback. Thanks!

@SamiSousa
Copy link
Contributor

@jkatz Could you push another 'dummy' commit? We just merged a fix for the CI issue we were having. Apologies for the inconvenience.

SamiSousa
SamiSousa suggested changes Mar 27, 2019
View reviewed changes
community-operators/postgresql/postgresql.package.yaml Outdated
@@ -1,4 +1,4 @@
packageName: postgresql
channels:
- name: alpha
- name: stable
Copy link
Contributor

Choose a reason for hiding this comment

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

For the record, this value need not match the maturity value in the CSV. This change is not required

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@SamiSousa Should I revert this or leave it be?

Copy link
Contributor

Choose a reason for hiding this comment

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

I recommend reverting it :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@SamiSousa Done 😄 And it will trigger a build, so we shall see how it goes!

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you also revert the change in the upstream's package.yaml as well? 😃

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@SamiSousa Done!

Updates for the Crunchy PostgreSQL Operator.
85a90ce 
This adds updates to the compatibility and name for the Crunchy PostgreSQL
Operator, as well as general cleanups to the page such as the icon and
links to the documentation.
@SamiSousa SamiSousa added the manual-test-needed Operator should be tested on cluster manually label Mar 28, 2019
@jkatz
Copy link
Contributor Author

jkatz commented Apr 9, 2019

Hi - I wanted to check in to see what else needs to be done on this patch.

Since last checking in, we've rolled out v3.5.2 of the Crunchy PostgreSQL Operator. I'd love to get this one completed so I can then submit the updated version. Thanks!

SamiSousa
SamiSousa approved these changes Apr 9, 2019
View reviewed changes
Copy link
Contributor

@SamiSousa SamiSousa 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-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 9, 2019
@SamiSousa SamiSousa merged commit 4e0dd0c into operator-framework:master Apr 9, 2019
@jkatz
Copy link
Contributor Author

jkatz commented Apr 9, 2019

Thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@SamiSousa SamiSousa SamiSousa approved these changes

@dmesser dmesser Awaiting requested review from dmesser

Assignees

@SamiSousa SamiSousa

Labels
lgtm Indicates that a PR is ready to be merged. manual-test-needed Operator should be tested on cluster manually needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@jkatz @openshift-ci-robot @dmesser @jomkz @robszumski @SamiSousa