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.

Sign up for GitHub

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

Jump to bottom

Stop deploying memcached automatically #145

Merged
merged 1 commit into from
Jul 3, 2023
Merged

Stop deploying memcached automatically #145

merged 1 commit into from
Jul 3, 2023

Conversation

kajinamit
Copy link
Contributor

@kajinamit kajinamit commented Jun 22, 2023 •
edited
Loading

... but require an pre-deployed instance. This follows the current design to accept mariadb database or rabbitmq.

Depends-on: openstack-k8s-operators/install_yamls#338

@openshift-ci openshift-ci bot requested review from abays and bshephar June 22, 2023 02:45
bshephar
bshephar reviewed Jun 23, 2023
View reviewed changes
Copy link
Collaborator

@bshephar bshephar left a comment

Choose a reason for hiding this comment

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

Not sure why kuttl is failing here. It seems to be complaining that there is no readyCount key:

case.go:366: resource Horizon:openstack/horizon: .status.readyCount: key is missing from map

controllers/horizon_controller.go Show resolved Hide resolved
@kajinamit
Copy link
Contributor Author

kajinamit commented Jun 23, 2023 •
edited
Loading

Not sure why kuttl is failing here. It seems to be complaining that there is no readyCount key:

I think this is because memached is not deployed. We have to merge openstack-k8s-operators/install_yamls#338 first.

@kajinamit
Copy link
Contributor Author

/retest

@kajinamit
Copy link
Contributor Author

/test horizon-operator-build-deploy-kuttl

@kajinamit
Stop deploying memcached automatically
4f00f36 
... but require an pre-deployed instance. This follows the current
design to accept mariadb database or rabbitmq.

Depends-on: openstack-k8s-operators/install_yamls#338
bshephar
bshephar approved these changes Jul 3, 2023
View reviewed changes
Copy link
Collaborator

@bshephar bshephar left a comment

Choose a reason for hiding this comment

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

Looks good to me. I noticed some of the other operators are starting to use namev1 instead of namev1beta1 now. We can probably look at changing that for Horizon too.

But this looks good. A nice addition, and a good culmination to all of the work you have put in to standardising the way we interact with memcached resources.

@openshift-ci openshift-ci bot added the lgtm label Jul 3, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 3, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bshephar, kajinamit

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

The pull request process is described 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-merge-robot openshift-merge-robot merged commit 956965e into openstack-k8s-operators:main Jul 3, 2023
3 checks passed
@kajinamit kajinamit deleted the mc-required branch July 4, 2023 00:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bshephar bshephar bshephar approved these changes

@abays abays Awaiting requested review from abays

Assignees

@bshephar bshephar

Labels
approved lgtm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kajinamit @bshephar @openshift-merge-robot