MCO-2006: Migrate pinned images sets private test cases #5466
Conversation
|
@sergiordlr: This pull request references MCO-2006 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 story to target the "4.21.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. |
openshift-ci-robot
added
the
jira/valid-reference
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sergiordlr 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 |
| // BeAvailable returns the gomega matcher to check if a resource is available or not. | ||
| func BeAvailable() types.GomegaMatcher { | ||
| return &DegradedMatcher{&conditionMatcher{conditionType: "Available", field: "status", expected: "True"}} | ||
| } |
There was a problem hiding this comment.
I didn't know it was so simple to define our own GomegaMatcher. Cool!
test/extended-priv/kubeletconfig.go
Outdated
| o.EventuallyWithOffset(1, &kc, timeout, "2s").Should(o.SatisfyAll( | ||
| HaveConditionField("Failure", "status", "False"), | ||
| HaveConditionField("Failure", "message", o.ContainSubstring(expectedMsg)), | ||
| ), "KubeletConfig '%s' should report Failure in status.conditions and report failure message %s. But it doesnt.", kc.GetName(), expectedMsg) |
There was a problem hiding this comment.
| ), "KubeletConfig '%s' should report Failure in status.conditions and report failure message %s. But it doesnt.", kc.GetName(), expectedMsg) | |
| ), "KubeletConfig '%s' should report Failure in status.conditions and report failure message %s. But it doesn't.", kc.GetName(), expectedMsg) |
Nit, but I do not think it is a blocker to letting this merge.
| # We trigger this service using a timeout with command: | ||
| # systemd-run --on-active="5minutes" --unit=tc-73659-clean-file-timed.service |
There was a problem hiding this comment.
Is this intentionally commented out?
There was a problem hiding this comment.
Yes. It explains how this unit is used. It is triggered by a timeout.
We first create the unit file, and instead of starting it directly, we run the commented command, so that it is triggered once the 5 minutes pass.
It tells the manual command used, just in case it is needed for debugging, or to understand how the test uses this unit.
The problem is that we cannot reach the node if it is under pressure because it rejects the debug pods. What we do is that we delay the file deletion 5 minutes, and then we put the node under pressure creating this file. Like that the pressure is auto-resolved (deleting the file) without we having to access the node again.
There was a problem hiding this comment.
I see, thank you for the detailed explanation!
|
|
||
| }) | ||
|
|
||
| // Disconnected clusters use an imagecontentsourcepolicy to mirror the images in openshittest. In this test cases we create an ImageDigestMirrorSet to mirror the same images and it is not supported |
There was a problem hiding this comment.
| // Disconnected clusters use an imagecontentsourcepolicy to mirror the images in openshittest. In this test cases we create an ImageDigestMirrorSet to mirror the same images and it is not supported | |
| // Disconnected clusters use an imagecontentsourcepolicy to mirror the images in openshifttest. In this test cases we create an ImageDigestMirrorSet to mirror the same images and it is not supported |
| DigestMirrorTest(oc, mcp, idmsName, idmsMirrors, pinnedImage, pinnedImageSetName) | ||
| }) | ||
|
|
||
| // Disconnected clusters use an imagecontentsourcepolicy to mirror the images in openshittest. In this test cases we create an ImageDigestMirrorSet to mirror the same images and it is not supported |
There was a problem hiding this comment.
| // Disconnected clusters use an imagecontentsourcepolicy to mirror the images in openshittest. In this test cases we create an ImageDigestMirrorSet to mirror the same images and it is not supported | |
| // Disconnected clusters use an imagecontentsourcepolicy to mirror the images in openshifttest. In this test cases we create an ImageDigestMirrorSet to mirror the same images and it is not supported |
| // Due to https://issues.redhat.com/browse/OCPBUGS-57473 we execute this test case in P1, and we have moved all OCL test cases out of P1 | ||
| // Like that we make sure that no test cases are skipped because the cluster is unhealthy. This change can be undone once the issue is fixed |
There was a problem hiding this comment.
This change can be undone once the issue is fixed
Is it worth making a Jira to track this follow up work? Or at least a comment on OCPBUGS-57473 to make sure this followup need is not lost?
There was a problem hiding this comment.
I added a comment here: https://issues.redhat.com/browse/OCPBUGS-57473?focusedId=28611858&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-28611858
We don't execute the test cases in defined groups here, like we do in the private repo. This comment doesn't make much sense in this repository. Sharding will be in charge of distributing them.
I'll re-phrase this comment in the code apart from adding the new comment in the jira ticket
| // TODO: When the "oc adm release info" command spec fails: | ||
| // 1. parse the output to get the release image name | ||
| // 2. search in all imagecontentsourcepolicies and all imagedigestimirrorsets if there is any mirror for the release image (it should) | ||
| // 3. use the mirror manually to get the image specs |
There was a problem hiding this comment.
Is this followup work that can be tracked in Jira?
There was a problem hiding this comment.
I created this ticket https://issues.redhat.com/browse/MCO-2008, included in the QE improvements epic.
| o.Expect(mcp.waitForPinComplete(waitForPinned)).To(o.Succeed(), "Pinned image operation is not completed in %s", mcp) | ||
| logger.Infof("OK!\n") | ||
|
|
||
| exutil.By("Check that only the image reamaining in the pinnedimageset is pinned") |
There was a problem hiding this comment.
| exutil.By("Check that only the image reamaining in the pinnedimageset is pinned") | |
| exutil.By("Check that only the image remaining in the pinnedimageset is pinned") |
yuqi-zhang
left a comment
yuqi-zhang
left a comment
There was a problem hiding this comment.
the general idea looks fine to me, some non-blocking thoughts on the suites inline
cmd/machine-config-tests-ext/main.go
Outdated
|
|
||
| // This suite will include all tests not included in the previous suite inheriting from openshift/disruptive | ||
| ext.AddGlobalSuite(e.Suite{ | ||
| Name: "openshift/machine-config-operator/qe", |
There was a problem hiding this comment.
Maybe we can name this something more descriptive, with something like slow or longrunning (haven't thought of a good name yet)
Alternatively we could add a Description here as well.
There was a problem hiding this comment.
We have configured this name: openshift/machine-config-operator/longduration and added a description for both suites.
cmd/machine-config-tests-ext/main.go
Outdated
| ext.AddGlobalSuite(e.Suite{ | ||
| Name: "openshift/machine-config-operator/qe", | ||
| Qualifiers: []string{ | ||
| `!name.contains("[Suite:openshift/machine-config-operator/disruptive")`, |
There was a problem hiding this comment.
Perhaps we should have an actual qualifier here, in case we want to add more suites in the future. Probably easier to be explicit than to depend on exclusion
There was a problem hiding this comment.
We use this qualifier now: [Suite:openshift/machine-config-operator/longduration]
isabella-janssen
left a comment
isabella-janssen
left a comment
There was a problem hiding this comment.
The tests look generally good to me. I agree with @yuqi-zhang's comment about the test suite name (#5466 (comment)) and I'd like to see some rehearsal payload runs, if possible.
sergiordlr
force-pushed
the
mco_2006_migrate_pinned_tests
branch
2 times, most recently
from
791058b to
3130a4f
Compare
|
@sergiordlr: 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. |
sergiordlr
force-pushed
the
mco_2006_migrate_pinned_tests
branch
from
3130a4f to
c7aa7a4
Compare
|
/payload-job periodic-ci-openshift-machine-config-operator-release-4.21-periodics-e2e-vsphere-mco-tp-longduration periodic-ci-openshift-machine-config-operator-release-4.21-periodics-e2e-aws-mco-tp-fips-proxy-longduration |
|
@sergiordlr: 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/d9f34dd0-d501-11f0-934c-989a16946a02-0 |
|
/payload-job periodic-ci-openshift-machine-config-operator-release-4.21-periodics-e2e-vsphere-mco-tp-longduration periodic-ci-openshift-machine-config-operator-release-4.21-periodics-e2e-aws-mco-fips-proxy-longduration |
|
@sergiordlr: trigger 2 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/1488b5c0-d502-11f0-9ab7-cf8e19f31771-0 |
|
/payload-job periodic-ci-openshift-machine-config-operator-release-4.21-periodics-e2e-vsphere-mco-tp-longduration periodic-ci-openshift-machine-config-operator-release-4.21-periodics-e2e-aws-mco-fips-proxy-longduration |
|
@sergiordlr: trigger 2 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/a153bb40-d529-11f0-81b6-b61ea4f4236f-0 |
|
/test e2e-gcp-op-single-node |
|
/payload-job-with-prs periodic-ci-openshift-machine-config-operator-release-4.21-periodics-e2e-aws-mco-fips-proxy-longduration openshift/origin#30592 |
|
@sergiordlr: 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/73bc4330-d5da-11f0-9bb4-9cb64c814439-0 |
4 participants
- What I did
Migrate pinnedimageset test cases from the private repository to MCO repository
A new suite
openshift/machine-config-operator/qehas been created. In this suite we will include all test cases not in cluded in the already existingopenshift/machine-config-operator/disruptivesuite. The idea is that one suite will contain the most important test cases and will be run more often, and the other test suites will contain the rest of the test cases and will be run less frequently.- How to verify it
All migrated test cases should pass