Skip to content
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

Automate [OLM] [OCP-21082] - [bz 1670311] Implement packages API server and list packagemanifest info with namespace not NULL #23772

Merged
merged 1 commit into from
Dec 13, 2019

Conversation

bandrade
Copy link
Contributor

@ecordell @njhale As title, please have a review. Thanks! cc: @cuipinghuo @scolange @chengzhang1016 @zihantang-rh @emmajiafan @jianzhangbjz

Success execution (when every package has namespace name defined)

./openshift-tests run all --dry-run | grep "Implement packages API server and list packagemanifest info with namespace not NULL" | ./openshift-tests run -f -
started: (0/1/1) "[Feature:Platform] OLM should Implement packages API server and list packagemanifest info with namespace not NULL [Suite:openshift/conformance/parallel]"

E0912 16:33:36.334871   15161 request.go:853] Unexpected error when reading response body: &http.httpError{err:"context deadline exceeded (Client.Timeout exceeded while reading body)", timeout:true}
Sep 12 16:33:32.681: INFO: >>> kubeConfig: /home/vagrant/kubeconfig
Sep 12 16:33:32.689: INFO: Waiting up to 30m0s for all (but 100) nodes to be schedulable
Sep 12 16:33:36.477: INFO: Waiting up to 10m0s for all pods (need at least 0) in namespace 'kube-system' to be running and ready
Sep 12 16:33:37.614: INFO: 0 / 0 pods in namespace 'kube-system' are running and ready (1 seconds elapsed)
Sep 12 16:33:37.614: INFO: expected 0 pod replicas in namespace 'kube-system', 0 are Running and Ready.
Sep 12 16:33:37.614: INFO: Waiting up to 5m0s for all daemonsets in namespace 'kube-system' to start
Sep 12 16:33:37.990: INFO: e2e test version: v0.0.0-master+$Format:%h$
Sep 12 16:33:38.356: INFO: kube-apiserver version: v1.14.6+13494ae
[BeforeEach] [Top Level]
  /home/vagrant/scripts/go/src/github.com/openshift/origin/test/extended/util/test.go:73
[BeforeEach] [Feature:Platform] OLM should
  /home/vagrant/scripts/go/src/github.com/openshift/origin/vendor/k8s.io/kubernetes/test/e2e/framework/framework.go:149
STEP: Creating a kubernetes client
Sep 12 16:33:38.356: INFO: >>> kubeConfig: /home/vagrant/kubeconfig
[BeforeEach] [Feature:Platform] OLM should
  /home/vagrant/scripts/go/src/github.com/openshift/origin/test/extended/util/client.go:109
Sep 12 16:33:41.797: INFO: configPath is now "/tmp/configfile995867157"
Sep 12 16:33:41.797: INFO: The user is now "e2e-test-olm-xrj6l-user"
Sep 12 16:33:41.797: INFO: Creating project "e2e-test-olm-xrj6l"
Sep 12 16:33:42.445: INFO: Waiting on permissions in project "e2e-test-olm-xrj6l" ...
Sep 12 16:33:42.842: INFO: Waiting for ServiceAccount "default" to be provisioned...
Sep 12 16:33:43.328: INFO: Waiting for ServiceAccount "deployer" to be provisioned...
Sep 12 16:33:43.811: INFO: Waiting for ServiceAccount "builder" to be provisioned...
Sep 12 16:33:44.297: INFO: Waiting for RoleBinding "system:image-pullers" to be provisioned...
Sep 12 16:33:45.077: INFO: Waiting for RoleBinding "system:image-builders" to be provisioned...
Sep 12 16:33:45.845: INFO: Waiting for RoleBinding "system:deployers" to be provisioned...
Sep 12 16:33:46.610: INFO: Project "e2e-test-olm-xrj6l" has been fully provisioned.
[It] Implement packages API server and list packagemanifest info with namespace not NULL [Suite:openshift/conformance/parallel]
  /home/vagrant/scripts/go/src/github.com/openshift/origin/test/extended/operators/olm.go:78
Sep 12 16:33:46.611: INFO: Running 'oc --config=/home/vagrant/kubeconfig get packagemanifest --all-namespaces -o=jsonpath={range .items[*]}{.metadata.name};{.status.catalogSourceNamespace}{"\n"}'
[AfterEach] [Feature:Platform] OLM should
  /home/vagrant/scripts/go/src/github.com/openshift/origin/test/extended/util/client.go:101
Sep 12 16:33:55.787: INFO: Deleted {user.openshift.io/v1, Resource=users  e2e-test-olm-xrj6l-user}, err: <nil>
Sep 12 16:33:56.163: INFO: Deleted {oauth.openshift.io/v1, Resource=oauthclients  e2e-client-e2e-test-olm-xrj6l}, err: <nil>
Sep 12 16:33:56.543: INFO: Deleted {oauth.openshift.io/v1, Resource=oauthaccesstokens  G-piszmiQKmVdMkDzTLXOQAAAAAAAAAA}, err: <nil>
[AfterEach] [Feature:Platform] OLM should
  /home/vagrant/scripts/go/src/github.com/openshift/origin/vendor/k8s.io/kubernetes/test/e2e/framework/framework.go:150
Sep 12 16:33:56.543: INFO: Waiting up to 3m0s for all (but 100) nodes to be ready
STEP: Destroying namespace "e2e-test-olm-xrj6l" for this suite.
Sep 12 16:34:04.806: INFO: Waiting up to 30s for server preferred namespaced resources to be successfully discovered
Sep 12 16:34:33.768: INFO: namespace e2e-test-olm-xrj6l deletion completed in 36.076239015s
Sep 12 16:34:33.801: INFO: Running AfterSuite actions on all nodes
Sep 12 16:34:33.811: INFO: Running AfterSuite actions on node 1

passed: (1m5s) 2019-09-12T16:34:33 "[Feature:Platform] OLM should Implement packages API server and list packagemanifest info with namespace not NULL [Suite:openshift/conformance/parallel]"


Timeline:

Sep 12 16:33:33.334 E kube-apiserver Kube API started failing: Get https://api.bandrade1.qe.devcluster.openshift.com:6443/api/v1/namespaces/kube-system?timeout=3s: net/http: request canceled (Client.Timeout exceeded while awaiting headers)
Sep 12 16:33:33.334 I openshift-apiserver OpenShift API started failing: Get https://api.bandrade1.qe.devcluster.openshift.com:6443/apis/image.openshift.io/v1/namespaces/openshift-apiserver/imagestreams/missing?timeout=3s: net/http: request canceled (Client.Timeout exceeded while awaiting headers)
Sep 12 16:33:35.287 I kube-apiserver Kube API started responding to GET requests
Sep 12 16:33:37.857 I openshift-apiserver OpenShift API started responding to GET requests
Sep 12 16:34:23.889 I ns/openshift-machine-api machine/bandrade1-l85l4-worker-ap-southeast-1b-ltv9s Updated machine bandrade1-l85l4-worker-ap-southeast-1b-ltv9s (165 times)
Sep 12 16:34:24.037 I ns/openshift-machine-api machine/bandrade1-l85l4-worker-ap-southeast-1c-fmbr6 Updated machine bandrade1-l85l4-worker-ap-southeast-1c-fmbr6 (165 times)
Sep 12 16:34:25.123 I ns/openshift-machine-api machine/bandrade1-l85l4-master-1 Updated machine bandrade1-l85l4-master-1 (161 times)
Sep 12 16:34:26.051 I ns/openshift-machine-api machine/bandrade1-l85l4-master-2 Updated machine bandrade1-l85l4-master-2 (162 times)
Sep 12 16:34:27.099 I ns/openshift-machine-api machine/bandrade1-l85l4-master-0 Updated machine bandrade1-l85l4-master-0 (161 times)
Sep 12 16:34:27.307 I ns/openshift-machine-api machine/bandrade1-l85l4-worker-ap-southeast-1a-pd967 Updated machine bandrade1-l85l4-worker-ap-southeast-1a-pd967 (165 times)

1 pass, 0 skip (1m5s)

@openshift-ci-robot openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Sep 12, 2019
@bandrade
Copy link
Contributor Author

/retest

@bandrade
Copy link
Contributor Author

/test e2e-cmd

Copy link
Contributor

@njhale njhale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding this. Just one comment:

test/extended/operators/olm.go Outdated Show resolved Hide resolved
@jianzhangbjz
Copy link
Contributor

/retest

@jianzhangbjz
Copy link
Contributor

@bandrade Got below errors, please have a check, thanks!

fail [github.com/openshift/origin/test/extended/operators/olm.go:81]: Unexpected error:
    <*util.ExitError | 0xc001a987e0>: {
        Cmd: "oc --config=/tmp/admin.kubeconfig get packagemanifest --all-namespaces --no-headers",
        StdErr: "Error from server: no table handler registered for this type *operators.PackageManifestList",
        ExitError: {

@bandrade
Copy link
Contributor Author

@jianzhangbjz it seems to be an issue with 4.3, so this test will aggregate when it's been merged =).

 oc get clusterversion
NAME      VERSION                        AVAILABLE   PROGRESSING   SINCE   STATUS
version   4.3.0-0.ci-2019-10-14-170839   True        False         32m     Cluster version is 4.3.0-0.ci-2019-10-14-170839
oc exec  catalog-operator-85c7c49fd7-lrmhd -n openshift-operator-lifecycle-manager -- olm -version
OLM version: 0.12.0
git commit: 0c9e4bf058b96a522e46e167c575d24745a15894

oc get packagemanifest
Error from server: no table handler registered for this type *operators.PackageManifestList"

Tracking this with https://bugzilla.redhat.com/show_bug.cgi?id=1761924

@bandrade
Copy link
Contributor Author

/retest

@bandrade
Copy link
Contributor Author

@njhale operator-framework/operator-lifecycle-manager#1078 was merged and now all jobs succeeded. Can you please review again? cc: @jianzhangbjz

o.Expect(err).NotTo(o.HaveOccurred())
for _, packageManifest := range strings.Split(msg, "\n") {
fields := strings.Fields(packageManifest)
if string(fields[0]) == " " && len(fields) < 4 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This checks that there's the expected number of fields - could we loop through them and verify that namespace is one of them instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was discussed previously on #23772 (comment) with @njhale. I'm verifying the number of fields due to one of them is optional(displayName), so the best approach that I found is to check when the line has less than 4 fields if the first character of the line is blank, which would check that namespace field was not returned. Please let me know if you have any suggestions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about something like this?

var packageserverLine string
for _, line := range strings.Split(msg, "\n") {
  if strings.Contains(line, "packageserver") {
    packageserverLine = line
  }
}
if !strings.Contains(packageserverLine, "openshift-operator-lifecycle-manager") {
  e2e.Failf("bad")
}

that way we look for a line we expect to be there, and ensure it has the correct value of the namespace?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, that will make this test to perform faster. Thank you for the suggestion

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed as requested, I just changed the packageserver to etcd and namespace to openshift-marketplace. The test passed locally. Thanks

@openshift-ci-robot openshift-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 27, 2019
@ecordell
Copy link
Contributor

ecordell commented Nov 4, 2019

@bandrade Why are there so many commits here? is this targeting the right base branch?

@bandrade
Copy link
Contributor Author

bandrade commented Nov 5, 2019

@ecordell I'm rebasing with the master, since this branch was created on Sep 12 there are a lot of commits after the branch was created.

@bandrade
Copy link
Contributor Author

bandrade commented Dec 2, 2019

@ecordell Is there anything that I should do to move on this PR? Thanks

@jianzhangbjz
Copy link
Contributor

@bandrade There are so many modified files. Could you help add your code based on the latest master branch? Thanks!

@openshift-ci-robot openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Dec 5, 2019
@bandrade
Copy link
Contributor Author

bandrade commented Dec 5, 2019

/retest

1 similar comment
@bandrade
Copy link
Contributor Author

bandrade commented Dec 5, 2019

/retest

@bandrade
Copy link
Contributor Author

bandrade commented Dec 5, 2019

@ecordell @jianzhangbjz , removed the wrong rebase, now it has only my changes and tests looks good. Can you please review? Thanks.

Copy link
Member

@kevinrizza kevinrizza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the most part this looks great. I just have some questions about some of the hard coded strings here.

test/extended/operators/olm.go Outdated Show resolved Hide resolved
test/extended/operators/olm.go Outdated Show resolved Hide resolved
@kevinrizza
Copy link
Member

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 5, 2019
@bandrade
Copy link
Contributor Author

bandrade commented Dec 5, 2019

/retest

1 similar comment
@bandrade
Copy link
Contributor Author

bandrade commented Dec 5, 2019

/retest

@bandrade
Copy link
Contributor Author

bandrade commented Dec 6, 2019

/test e2e-aws-fips

@bandrade
Copy link
Contributor Author

bandrade commented Dec 6, 2019

/retest

@bandrade
Copy link
Contributor Author

bandrade commented Dec 6, 2019

/test e2e-aws-fips

@bandrade
Copy link
Contributor Author

bandrade commented Dec 6, 2019

@bparees Could you help give approval? Thanks!

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Dec 12, 2019
@bparees
Copy link
Contributor

bparees commented Dec 12, 2019

/approve

@bandrade please squash your commits and get someone from the olm-team to lgtm.

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 12, 2019
@bandrade
Copy link
Contributor Author

/retest

@bandrade bandrade requested a review from bparees December 12, 2019 23:14
@kevinrizza
Copy link
Member

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 13, 2019
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bandrade, bparees, kevinrizza

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

1 similar comment
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-ci-robot
Copy link

openshift-ci-robot commented Dec 13, 2019

@bandrade: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
ci/prow/e2e-aws-upgrade c99d5ad link /test e2e-aws-upgrade

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.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants