-
Notifications
You must be signed in to change notification settings - Fork 31
OCPBUGS-54877: Add catalogd-cas-dir option to op-con #316
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
OCPBUGS-54877: Add catalogd-cas-dir option to op-con #316
Conversation
Signed-off-by: Todd Short <todd.short@me.com>
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tmshort 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 |
- op: add | ||
path: /spec/template/spec/containers/0/env | ||
value: [{"name":"SSL_CERT_DIR", "value":"/var/ca-certs"}] |
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.
Kinda feel like we should take this out of the manifests since we're providing a nearly-equivalent (but better) option as part of this PR.
If SSL_CERT_FILE is being used for container/images, then does it need to also be updated to be able to read updates to those paths successfully?
Because of https://github.com/golang/go/blob/master/src/crypto/x509/cert_pool.go?name=release#L116 ?
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.
I'm reluctant to, since it's still being used as a base for the SystemCertPool, and thus used by container/images.
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.
And setting the --pull-cas-dir
completely changes the behavior (we lose additionalTrustedCA support)
And I just realized, the SSL_CERTS_DIR is needed for running our upstream e2e testing using a local repo that is a local service
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.
I feel like we should copy some of this out-of-codebase, out-of-mind knowledge into this file as comments to explain to our future selves:
- What the heck all this is for
- Any gotchas that we know about, so that we avoid tripping over them in the future
If folks agree with that, it'd be maybe better in a follow-up once we've got it all figured out and we're not in fire drill mode.
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.
There's already something documented in Google Docs.
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.
My main thing is having a breadcrumb here that helps us make sense if what's here. Can we link the google doc here?
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.
Yes... but followup?
@tmshort: This pull request explicitly references no jira issue. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
/payload-aggregate periodic-ci-openshift-multiarch-master-nightly-4.19-ocp-e2e-azure-ovn-multi-x-ax 10 |
@tmshort: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/1b33d920-1a1a-11f0-8c5f-ee739d333a54-0 |
/payload-aggregate periodic-ci-openshift-release-master-nightly-4.19-e2e-vsphere-ovn 10 |
@tmshort: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/1fb394e0-1a1a-11f0-8220-72a13906a7d1-0 |
/retitle OCPBUGS-54877: Add catalogd-cas-dir option to op-con |
@tmshort: This pull request references Jira Issue OCPBUGS-54877, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: The bug has been updated to refer to the pull request using the external bug tracker. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
the e2e-aws-techpreview job failed but I don't see the "x509" error, which was reliably there in the previous failures. Retesting after a while since both e2e-aws and e2e-techpreview failed with:
which looks like aws cluster provisioning issue |
/retest-required |
/test openshift-e2e-aws-techpreview |
/test openshift-e2e-aws |
/payload-aggregate-with-prs periodic-ci-openshift-multiarch-master-nightly-4.19-ocp-e2e-azure-ovn-multi-x-ax 5 #316 |
@tmshort: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/a34b5390-1a3f-11f0-9ec5-1d03e7a8039c-0 |
@tmshort: This PR was included in a payload test run from #316
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/a34b5390-1a3f-11f0-9ec5-1d03e7a8039c-0 |
/payload-aggregate periodic-ci-openshift-release-master-nightly-4.19-e2e-vsphere-ovn 10 |
@tmshort: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/af65c6b0-1a3f-11f0-9286-4615665673f7-0 |
/retest-required |
Trigger a test via the cluster-bot:
|
/test openshift-e2e-aws-techpreview |
The |
Test pass, see: https://issues.redhat.com/browse/OCPBUGS-54877 |
@tmshort: This pull request references Jira Issue OCPBUGS-54877, which is valid. 3 validation(s) were run on this bug
No GitHub users were found matching the public email listed for the QA contact in Jira (jiazha@redhat.com), skipping review request. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
@tmshort: The following test failed, say
Full PR test history. Your PR dashboard. 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-sigs/prow repository. I understand the commands that are listed here. |
ad55882
into
openshift:main
@tmshort: Jira Issue OCPBUGS-54877: Some pull requests linked via external trackers have merged: The following pull requests linked via external trackers have not merged: These pull request must merge or be unlinked from the Jira bug in order for it to move to the next state. Once unlinked, request a bug refresh with Jira Issue OCPBUGS-54877 has not been moved to the MODIFIED state. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
[ART PR BUILD NOTIFIER] Distgit: ose-olm-catalogd |
[ART PR BUILD NOTIFIER] Distgit: ose-olm-operator-controller |
/cherry-pick release-4.18 |
@tmshort: #316 failed to apply on top of branch "release-4.18":
In response to this:
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-sigs/prow repository. |
Will have to cherry-pick manually. |
This forces us to explicitly watch the
/var/ca-certs
dir, and properly reload the certs when thex509.SystemCertPool()
may not.