Marketplace OPSRC&CSC with publisher&displayName #23711
Conversation
openshift-ci-robot
added
the
size/L
|
/retest |
There was a problem hiding this comment.
Can you more clearly state the purpose of this test? What labels are we trying to test and why? It seems like we are testing that custom labels are propogated to child resources, but we are also testing that the generated ones are? Should those maybe be separate tests?
And why are we checking the packagemanifest for labels? That may also be a valid test, but it seems like it should also maybe be a separate one.
| allNs = "openshift-operators" | ||
| marketplaceNs = "openshift-marketplace" | ||
|
|
||
| //buildPruningBaseDir = exutil.FixturePath("testdata", "marketplace") |
There was a problem hiding this comment.
Should this commented out code be removed?
| g.It("[ocp-21728]create opsrc with labels", func() { | ||
|
|
||
| //create one opsrc with label | ||
| //opsrcYaml := exutil.FixturePath("testdata", "marketplace", "opsrc", "02-opsrc.yaml") |
There was a problem hiding this comment.
Same question about commented code
| //get the packagelist with label of opsrctestlabel | ||
| packageListOpsrc2, _ := oc.AsAdmin().WithoutNamespace().Run("get").Args("packagemanifests", "-lopsrc-provider=optestlabel", "-o=name", "-n", marketplaceNs).Output() | ||
| for _, packages := range packageList { | ||
| o.Expect(packageListOpsrc2).Should(o.ContainSubstring(packages)) |
There was a problem hiding this comment.
I'm confused by this last part. I thought the purpose of this test was to check label propogation on marketplace child resources? Why are we checking the packagemanifest here? Or am I misunderstanding the purpose of this test?
There was a problem hiding this comment.
This test will check label propogation on marketplace child resources and the packagemanifests from the child "catalogsource" ,The packcagemanifests will alsoo attache the label and they can be filtered by the label.
| - name: MARKETPLACE | ||
| - name: PACKAGES | ||
| - name: DISPLAYNAME | ||
| - name: PUBLISHER |
There was a problem hiding this comment.
This needs a newline at the end of the file.
| - name: MARKETPLACE | ||
| - name: LABEL | ||
| - name: DISPLAYNAME | ||
| - name: PUBLISHER |
There was a problem hiding this comment.
This also needs a newline at the end of the file.
|
/retest |
|
@emmajiafan: Cannot trigger testing until a trusted user reviews the PR and leaves an 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/test-infra repository. |
|
/ok-to-test |
openshift-ci-robot
added
the
ok-to-test
| cscYaml, err := oc.AsAdmin().Run("process").Args("--ignore-unknown-parameters=true", "-f", cscYamltem, "-p", "NAME=csctestlabel", fmt.Sprintf("NAMESPACE=%s", allNs), fmt.Sprintf("MARKETPLACE=%s", marketplaceNs), "PACKAGES=descheduler-test", "DISPLAYNAME=csctestlabel", "PUBLISHER=csctestlabel").OutputToFile("config.json") | ||
| err = createResources(oc, cscYaml) | ||
| o.Expect(err).NotTo(o.HaveOccurred()) | ||
| time.Sleep(15 * time.Second) |
There was a problem hiding this comment.
Here, the same as the above.
There was a problem hiding this comment.
Update to use wait.Poll
|
|
||
| err = createResources(oc, opsrcYaml) | ||
| o.Expect(err).NotTo(o.HaveOccurred()) | ||
| time.Sleep(30 * time.Second) |
There was a problem hiding this comment.
@emmajiafan It's better to use wait.Poll instead of the Sleep function.
There was a problem hiding this comment.
Update to use wait.Poll
| return true, nil | ||
| } | ||
|
|
||
| func isResourceItemsEmpty(resourceList map[string]interface{}) bool { |
There was a problem hiding this comment.
@emmajiafan Do we still need to use this function? If not, please remove it and other functions which we don't need.
|
/retest |
|
/lgtm |
|
Hi @knobunc , could you help add "approved" label to make this pr merged? Thanks |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: emmajiafan, kevinrizza, knobunc 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 |
openshift-ci-robot
added
the
approved
|
/retest Please review the full test history for this PR and help us cut down flakes. |
2 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
11 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@emmajiafan: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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/test-infra repository. I understand the commands that are listed here. |
Auto [OCP-21728 check the publisher,display,labels of opsrc&csc] @kevinrizza @ecordell Could you help review it? Thanks very much! The test case covers OCP-21728 for marketplace. cc: @bandrade @scolange @cuipinghuo @dongboyan77 @zihantang-rh @jianzhangbjz @chengzhang1016
The logs as below:
passed: (1m33s) 2019-09-02T16:22:38 "[Feature:Marketplace] Marketplace resources with labels provider displayName [ocp-21728]create opsrc with labels [Suite:openshift/conformance/parallel]"
Timeline:
Sep 02 16:21:15.028 I ns/openshift-marketplace deployment/opsrctestlabel Scaled up replica set opsrctestlabel-76fcd58f7d to 1
Sep 02 16:21:15.028 I ns/openshift-marketplace replicaset/opsrctestlabel-76fcd58f7d Created pod: opsrctestlabel-76fcd58f7d-nbqjc
Sep 02 16:21:15.028 I ns/openshift-marketplace pod/opsrctestlabel-76fcd58f7d-nbqjc Successfully assigned openshift-marketplace/opsrctestlabel-76fcd58f7d-nbqjc to ip-10-0-154-110.ap-northeast-2.compute.internal
Sep 02 16:21:15.037 I ns/openshift-marketplace pod/opsrctestlabel-76fcd58f7d-nbqjc node/ created
Sep 02 16:21:16.559 I ns/openshift-marketplace pod/opsrctestlabel-76fcd58f7d-nbqjc Container image "quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:1ac6e4de4a360f1d33e141aa06f968de71cb4dfbe88711bcf35995f12a06c654" already present on machine
...
Sep 02 16:22:37.889 I ns/openshift-marketplace pod/opsrctestlabel-76fcd58f7d-nbqjc Stopping container opsrctestlabel
Sep 02 16:22:38.735 W ns/openshift-marketplace pod/csctestlabel-6f7dbdcb9d-9jxbr node/ip-10-0-154-110.ap-northeast-2.compute.internal graceful deletion within 30s
Sep 02 16:22:38.746 I ns/openshift-marketplace pod/csctestlabel-6f7dbdcb9d-9jxbr Stopping container csctestlabel
Writing JUnit report to junit_e2e_20190902-162238.xml
1 pass, 0 skip (1m33s)