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

feat: upgrade PO to 0.61.0-rhobs1 #234

Merged
merged 4 commits into from
Dec 14, 2022
Merged

Conversation

sthaha
Copy link
Collaborator

@sthaha sthaha commented Dec 7, 2022

No description provided.

@sthaha sthaha force-pushed the feat-upgrade-po-0.61.1 branch from e348df8 to 91a724d Compare December 7, 2022 05:13
Copy link
Contributor

@simonpasquier simonpasquier left a comment

Choose a reason for hiding this comment

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

The failure might be related to the fact that the admission webhook's service inside the pod is now exposed on port 443 rather than 8443?

@sthaha sthaha force-pushed the feat-upgrade-po-0.61.1 branch from 91a724d to 11f62ba Compare December 9, 2022 03:38
@sthaha
Copy link
Collaborator Author

sthaha commented Dec 9, 2022

@simonpasquier

The failure might be related to the fact that the admission webhook's service inside the pod is now exposed on port 443 rather than 8443?

Thank you for the pointer. I had noticed it. The issue was that I was modifying only the prometheusrule validating webhook to adapt to the changes to admission-webhook but not the alertmanager-config validating webhook and the service generated by OLM resulted in conflict.

I have fixed that now by

  • using the upstream service.yaml
  • adding a comment to the future maintainers to modify both validating-webhooks and not just one

@sthaha sthaha force-pushed the feat-upgrade-po-0.61.1 branch from 11f62ba to 2a692dc Compare December 9, 2022 08:02
@sthaha sthaha marked this pull request as ready for review December 9, 2022 08:20
@sthaha sthaha requested a review from a team as a code owner December 9, 2022 08:20
@sthaha sthaha enabled auto-merge (squash) December 9, 2022 08:24
@sthaha sthaha disabled auto-merge December 9, 2022 08:24
jan--f
jan--f previously approved these changes Dec 9, 2022
Copy link
Collaborator

@jan--f jan--f left a comment

Choose a reason for hiding this comment

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

👍 Thanks!

name: obo-prometheus-operator-admission-webhook
namespace: operators
path: /admission-prometheusrules/validate
port: 8443
failurePolicy: Ignore
failurePolicy: Fail
Copy link
Contributor

Choose a reason for hiding this comment

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

I wouldn't change the failure policy as part of this PR. 'Fail' means that applying PrometheusRule will be rejected when the admission webhook service is unavailable or unreachable. We haven't found a good solution for this in CMO for instance.

Admission Webhook Deployment relies on certs injected by OLM, thus this
commit removes tls-certificates volume mounts from admission-webhook
deployment.

Signed-off-by: Sunil Thaha <sthaha@redhat.com>
@sthaha sthaha force-pushed the feat-upgrade-po-0.61.1 branch from 00b4060 to af170a3 Compare December 10, 2022 03:53
@jan--f jan--f merged commit 8f342e8 into rhobs:main Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants