OPRUN-3863: Add olmv1 webhook support origin tests#29825
OPRUN-3863: Add olmv1 webhook support origin tests#29825perdasilva wants to merge 1 commit intoopenshift:mainfrom
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
perdasilva
force-pushed
the
olmv1-webhook-tests
branch
from
1816e5d to
c7e942a
Compare
|
/test e2e-metal-ipi-ovn-dualstack-bgp-techpreview |
perdasilva
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@perdasilva: This pull request references OPRUN-3863 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.20.0" version, but no target version was set. DetailsIn 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. |
perdasilva
force-pushed
the
olmv1-webhook-tests
branch
4 times, most recently
from
2a43c3a to
e1e5feb
Compare
|
/test e2e-metal-ipi-ovn-dualstack-bgp-techpreview |
|
Job Failure Risk Analysis for sha: e1e5feb
|
perdasilva
force-pushed
the
olmv1-webhook-tests
branch
2 times, most recently
from
d3c2644 to
44a9c22
Compare
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
|
/test e2e-metal-ipi-ovn-dualstack-bgp-techpreview |
|
@perdasilva: This pull request references OPRUN-3863 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.20.0" version, but no target version was set. DetailsIn 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. |
|
/retest |
|
Job Failure Risk Analysis for sha: 44a9c22
|
| certificateSecretName = "webhook-operator-webhook-service-cert" | ||
| ) | ||
|
|
||
| var _ = g.Describe("[sig-olmv1][OCPFeatureGate:NewOLMWebhookProviderOpenshiftServiceCA][Skipped:Disconnected][Serial] OLMv1 operator with webhooks", g.Ordered, g.Serial, func() { |
There was a problem hiding this comment.
I wonder if there is some way to test certificate rotation and check that the service is available after a rotation occurs? I feel like that has been problematic in OLMv0's implementation.
There was a problem hiding this comment.
That's a great question, and I'm really not sure how much we can control openshift-serviceca's rotation period. Let me poke at it.
There was a problem hiding this comment.
I've updated the two tests:
- ensuring service availability during cert secret deletion by continuously creating and deleting a resource after the deletion of the cert secret
- ensuring deployment availability during cert rotation, which is forced to happen when the openshift-service-ca signing key gets deleted. The problem I ran into is that then the certificate is no longer signed and fails for another reason. So, I'm not sure the best way to test this case =S maybe we just cannot...
There was a problem hiding this comment.
For the latter case, I think my expectation would be:
- If service-ca signing key is deleted, then service-ca-operator creates a new signing key and then rotates all certs.
- When service-ca-operator creates a new signing key, the webhook configurations are updated to reflect the new key and the kubernetes API server starts using that key to validate the webhook servers.
- When service-ca-operator rotates the webhook server's cert (so that it is signed by the new signing key), the server pods notice that change and start using that new cert.
2 and 3 wouldn't happen atomically at the same time, so I think it would be safe to assume some downtime, between catalogd and operator-controller, but then for that to correct itself on its own after both (2) and (3) occur.
Is that your understanding of what should happen?
There was a problem hiding this comment.
I think that's reasonable. I was wondering if it would be a controlled hand-off to the new pod by the deployment.
I.e. when the tls secret changes, deployment notices, rotates the pods by bringing up the new, switching traffic, then bringing down the old one. But, it doesn't sound unreasonable that there might be some downtime.
There was a problem hiding this comment.
The service-ca validity period is 26 months, a time period where users are expected to have upgraded, such that the certificate is rotated as part of an upgrade.
perdasilva
force-pushed
the
olmv1-webhook-tests
branch
from
44a9c22 to
51a650d
Compare
|
Job Failure Risk Analysis for sha: 51a650d
|
perdasilva
force-pushed
the
olmv1-webhook-tests
branch
from
51a650d to
36008d8
Compare
|
/test e2e-metal-ipi-ovn-dualstack-bgp-techpreview |
perdasilva
force-pushed
the
olmv1-webhook-tests
branch
from
c7bb08c to
e9b29a9
Compare
openshift-merge-robot
removed
the
needs-rebase
perdasilva
force-pushed
the
olmv1-webhook-tests
branch
from
e9b29a9 to
d88423b
Compare
|
/payload-job periodic-ci-openshift-release-master-ci-4.19-e2e-aws-ovn-techpreview-serial |
|
@perdasilva: 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/bde39b50-60b6-11f0-8574-1255cecb9182-0 |
perdasilva
force-pushed
the
olmv1-webhook-tests
branch
from
d88423b to
c9e6602
Compare
|
/payload-job periodic-ci-openshift-release-master-ci-4.20-e2e-aws-ovn-techpreview |
|
@perdasilva: 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/6a587380-60c9-11f0-9e53-2a134413be2d-0 |
|
@perdasilva: 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/22d2d510-60cd-11f0-9076-ff16c0ab5472-0 |
perdasilva
force-pushed
the
olmv1-webhook-tests
branch
from
c9e6602 to
f8a8aa0
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: perdasilva, thetechnick The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
openshift-merge-robot
added
the
needs-rebase
perdasilva
force-pushed
the
olmv1-webhook-tests
branch
from
f8a8aa0 to
15aa5f8
Compare
openshift-merge-robot
removed
the
needs-rebase
Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>
perdasilva
force-pushed
the
olmv1-webhook-tests
branch
from
15aa5f8 to
cce8714
Compare
|
/payload-job periodic-ci-openshift-release-master-ci-4.20-e2e-aws-ovn-techpreview |
|
@perdasilva: 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/ad7c5930-6c70-11f0-881b-c69e0b18b9cb-0 |
|
/retest-required |
|
/test e2e-gcp-ovn-builds |
|
@perdasilva: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
|
All required tests are passing! |
| defer g.GinkgoRecover() | ||
| oc := exutil.NewCLIWithoutNamespace("openshift-operator-controller-webhooks") | ||
|
|
||
| g.BeforeAll(func(ctx g.SpecContext) { |
There was a problem hiding this comment.
You cannot use beforeall like this in origin, it's not true ginkgo. Every test has to be completely independent because they are run in their own process -- so BeforeAll effectively becomes BeforeEach.. If you need a cluster configuration the job would be dedicated to this purpose, configure it the right way and then run your suite.
OTE has designs for tests tied to configs, but it's not implemented yet. https://github.com/openshift/enhancements/blob/master/enhancements/testing/openshift-tests-extension.md
|
/close |
openshift-merge-robot
added
the
needs-rebase
|
PR needs rebase. DetailsInstructions 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. |
|
@anik120: Closed this PR. DetailsIn 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. |
8 participants
Adds origin tests for the webhook support feature. Test need to be run serially to enable BeforeSuite functionality which installs the webhook operator for the tests:
(I'm not sure I'd add the last two if it weren't for the need to have 5 tests)