Skip to content

Conversation

@damemi
Copy link

@damemi damemi commented Apr 16, 2020

This implements the run-resourcewatch config change watcher for kube-apiserver-operator's e2e (openshift/origin#24845)

@damemi
Copy link
Author

damemi commented Apr 16, 2020

/retest

Copy link
Contributor

Choose a reason for hiding this comment

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

Are you tarring while the writer is still running? You need to stop it first as was suggested in the Slack thread.

Copy link
Author

Choose a reason for hiding this comment

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

I was wondering about that, the only way I can think to stop the controller is by killing the process because it doesn't really have a "stop" condition, so we can't just wait for it to end. After the e2e test runs, there shouldn't be any more changes to the configs right?
@mfojtik @sanchezl

@damemi damemi force-pushed the resourcewatch-test branch from cfd6e36 to b545146 Compare April 17, 2020 13:50
@damemi
Copy link
Author

damemi commented Apr 17, 2020

/retest

1 similar comment
@damemi
Copy link
Author

damemi commented Apr 21, 2020

/retest

openshift-tests run-resourcewatch &
rw_pid=$!
make test-e2e JUNITFILE=/tmp/artifacts/junit_report.xml --warn-undefined-variables
kill -9 $rw_pid
Copy link
Contributor

Choose a reason for hiding this comment

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

graceful kill doesn't work?

@deads2k
Copy link
Contributor

deads2k commented Apr 21, 2020

/lgtm
/hold

you need green proof

@damemi if this works, how do we do this enable it for all jobs?

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 21, 2020
@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 21, 2020
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: damemi, deads2k

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details 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-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 21, 2020
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Apr 21, 2020

@damemi: The following tests failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/rehearse/openshift/cluster-kube-apiserver-operator/master/e2e-aws-operator db28946 link /test pj-rehearse
ci/prow/pj-rehearse db28946 link /test pj-rehearse

Full PR test history. Your PR dashboard.

Details

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.

@damemi
Copy link
Author

damemi commented Apr 21, 2020

Seeing this error when it tries to run: /bin/bash: line 77: openshift-tests: command not found. @tnozicka or @sanchezl any idea where the openshift-tests binary would be in this case?

@damemi
Copy link
Author

damemi commented Apr 29, 2020

Fixing the missing binary test will rely on openshift/cluster-kube-apiserver-operator#844

@damemi
Copy link
Author

damemi commented May 13, 2020

This is still blocked on the discussion in openshift/cluster-kube-apiserver-operator#844 (comment)

Specifically, we need to make the binary available in the operator image by copying it from the builder. But we only want to have it available in CI. I couldn't find any way to conditionally do the docker COPY so if we can't figure that out, we'll either need to always copy the openshift-tests binary or do this somewhere besides the operator e2e

@deads2k
Copy link
Contributor

deads2k commented May 19, 2020

This is still blocked on the discussion in openshift/cluster-kube-apiserver-operator#844 (comment)

Specifically, we need to make the binary available in the operator image by copying it from the builder. But we only want to have it available in CI. I couldn't find any way to conditionally do the docker COPY so if we can't figure that out, we'll either need to always copy the openshift-tests binary or do this somewhere besides the operator e2e

put in the image in the openshift-test image, right?

Copy link
Author

@damemi damemi left a comment

Choose a reason for hiding this comment

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

/close

@openshift-ci-robot
Copy link
Contributor

@damemi: Closed this PR.

Details

In response to this:

/close

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.

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. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants