(HOLD) OPRUN-3941: Add webhook tests with enhancements and fixes#430
(HOLD) OPRUN-3941: Add webhook tests with enhancements and fixes#430camilamacedo86 wants to merge 1 commit intoopenshift:mainfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: camilamacedo86 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 |
158497e to
44e43f7
Compare
44e43f7 to
9385026
Compare
|
@camilamacedo86: This pull request references OPRUN-3941 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 epic 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. |
| } | ||
|
|
||
| func newWebhookTestV1(name, namespace string, valid bool) *unstructured.Unstructured { | ||
| func newWebhookTest(name, namespace string, valid bool) *unstructured.Unstructured { |
There was a problem hiding this comment.
Again, this is not anything major at all, it was just bothering me that this function returns a webhook, so that we can use that in our tests, so newTestWebhook seems more of an appropriate name, than newWebhookTest. newWebhookTest means this is a test itself, which it is not.
There was a problem hiding this comment.
the common standard is New<StructName|Kind|Class>
In this case, the kind inside of the sample/project that we use to do the test is "kind": "WebhookTest",
so, therefore, make sense newWebhookTest
There was a problem hiding this comment.
we are asking for a NEW struct of WebhookTest
There was a problem hiding this comment.
| fmt.Fprintf(GinkgoWriter, "\n[pod-logs] namespace=%s\n", namespace) | ||
|
|
||
| By("Getting all pods in the namespace") | ||
| namesOut, err := RunK8sCommand(ctx, "get", "pods", "-n", namespace, "-o", "name") |
There was a problem hiding this comment.
Do we need a cli tool (kubectl/oc) to get pod logs? Can't we just use controller runtime client to get all the pods?
There was a problem hiding this comment.
QE tests will need to use the CLI so here we add the helper already for them
And in this case is more cleaner the code with oc/kubectl then with the API
I mean, the code is KISS. Otherwise, we need add more extra deps
9385026 to
8e962e9
Compare
|
/test openshift-e2e-aws |
1 similar comment
|
/test openshift-e2e-aws |
8e962e9 to
96c2e91
Compare
96c2e91 to
3d34d5a
Compare
3d34d5a to
e25dccd
Compare
|
Test #436 is a cert fix for our upstream-e2e, and has nothing to do with OTE, so I'm not sure why you're referencing it? |
|
/hold |
Because the test that we faced issues in Sippy is Therefore, I wanted to test it with your PR. |
a4fe57d to
57ab057
Compare
|
/payload-aggregate periodic-ci-openshift-release-master-ci-4.20-e2e-gcp-ovn-techpreview-serial 10 |
|
@camilamacedo86: 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/9db3f980-7a77-11f0-9317-77fddb6f96d1-0 |
57ab057 to
60f247f
Compare
|
/test openshift-e2e-aws |
|
/payload-aggregate periodic-ci-openshift-release-master-ci-4.20-e2e-gcp-ovn-techpreview-serial 10 (we need check if after the cert-fix it is either fixed) |
|
@camilamacedo86: 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/389f9120-7bf9-11f0-8801-38b19603745e-0 |
60f247f to
fa0cb6e
Compare
… certificate rotation This change is a refactor of code from openshift/origin#30059. Assisted-by: Gemini
fa0cb6e to
b736e1b
Compare
|
@camilamacedo86: This pull request references Jira Issue OCPBUGS-60564, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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. |
|
@camilamacedo86: This pull request references OPRUN-3941 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 epic 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. |
|
Closing I will open a new PR to faciliate the review and analise with the test that we do not add due the issues faced, more info: https://redhat-internal.slack.com/archives/C06KP34REFJ/p1755607157127779?thread_ts=1755602572.169319&cid=C06KP34REFJ |
From: #434
- Skips the test that is failing. We will fix it in a follow-up. We need #436 in the payload.
- Add dumping of container logs and
kubectl describe podsoutput for better diagnostics.- Include targeted certificate details dump (
tls.crtparse) when failures occur.- Add additional check to verify webhook responsiveness after certificate rotation.
This change is a refactor with fixes of the code from openshift/origin#30059.
/payload-aggregate periodic-ci-openshift-release-master-ci-4.20-e2e-gcp-ovn-techpreview-serial 10
Testhing with payload and GCP to ensure that the error: https://prow.ci.openshift.org/view/gs/test-platform-results/logs/periodic-ci-shiftstack-ci-release-4.20-techpreview-e2e-openstack-ovn-serial-techpreview/1955816149002227712
Will not be faced