-
Notifications
You must be signed in to change notification settings - Fork 82
Update setup_singleton.sh to support F5 Cert Manager #2626
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
base: scripts-dev
Are you sure you want to change the base?
Conversation
pgodowski
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some clarifying question asked, @aaronmcohen pls clarify
| info "Checking cert manager readiness." | ||
| #check webhook pod runnning | ||
| local name="cert-manager-webhook" | ||
| local f5_exclude_name="f5-cert-manager-webhook" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this a confirmed name, could it be customized at F5 cert manager install time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pgodowski This is based on F5's documentation https://clouddocs.f5.com/service-proxy/latest/spk-cert-manager.html. I don't see any option to customize it. That being said somewhere in the universe will be a admin who chooses to edit the contents of F5's helm chart. Based on that concern... Should name and f5_exclude_name pull from an environment variable?
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: aaronmcohen, pgodowski The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Personally I think this script needs some more work - for example, not only f5 - we need to allow ANY cert-manager that has the same code base (community edition); otherwise, we will be back here again. Is there truly a very specific protocol in watsonx software that can not be handled by any cert-manager, with the correct configuration, i.e. watching distinct namespaces to avoid collision? So ideally, the script will 1) check for more than 1 cert manager 2) examine the namespace scope, annotations, and labels, and look for collision then 3) if there is a collision, THEN fail 4) if no collision, please let us get on with the upgrade ;-) |
@kc0olm I agree the script could be improved. However could we separate that effort from this PR which is to unblock Verizon? |
|
@pgodowski Thanks for working on this. Unfortunately I need a hotfix or workaround right now - I am all done upgrading Verizon - except for the Licensing Server, which won't upgrade if it finds two cert-managers. Actually, it causes another embarassing problem where we had to go deep into the yaml files and add a line in several places to indicate the "custom" certmanager. It's just not good to be editing Operator code on the fly, with the customer watching. https://github.ibm.com/PrivateCloud-analytics/Zen/issues/43592 |
What this PR does / why we need it:
The F5 certificate manager is a fork of the community certificate manager which should be ignored by the script as it confined to a single namespace and uses different CRDs.
https://clouddocs.f5.com/service-proxy/latest/spk-cert-manager.html
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
How to backport this PR to other branch:
backport <branch-name>backport <branch-name>and leave a comment/backportto trigger the backport action