Hook up resource watch to one job #33109
Conversation
|
/payload-job periodic-ci-openshift-release-master-ci-4.12-upgrade-from-stable-4.11-e2e-aws-ovn-upgrade |
|
@xueqzhan: the repo openshift/release does not contribute to the OpenShift official images |
openshift-ci
bot
added
the
do-not-merge/invalid-owners-file
xueqzhan
force-pushed
the
resource-watch-observer
branch
from
5f500e6
to
2a716ff
Compare
openshift-ci
bot
removed
the
do-not-merge/invalid-owners-file
xueqzhan
force-pushed
the
resource-watch-observer
branch
2 times, most recently
from
3ee322e
to
37aa374
Compare
|
/payload-job periodic-ci-openshift-release-master-ci-4.12-upgrade-from-stable-4.11-e2e-aws-ovn-upgrade |
|
@xueqzhan: the repo openshift/release does not contribute to the OpenShift official images |
|
/pj-rehearse |
|
/test pj-rehearse |
1 similar comment
|
/test pj-rehearse |
@danilo-gemoli Thanks for the link! Maybe I am missing something. I do not see how to define timeout for observer in that link. I also posted the question on the slack: https://coreos.slack.com/archives/CBN38N3MW/p1665769983392569. Feel free to communicate at the place of your preference. |
xueqzhan
force-pushed
the
resource-watch-observer
branch
5 times, most recently
from
bc147d5
to
0ce6484
Compare
|
|
||
| echo "ended resource watch gracefully" | ||
| } | ||
| trap cleanup SIGTERM |
There was a problem hiding this comment.
May I suggest to replace this with trap cleanup EXIT?
There was a problem hiding this comment.
That worked! Thanks!
xueqzhan
force-pushed
the
resource-watch-observer
branch
from
0ce6484
to
17a0c28
Compare
|
@dgoodwin We should be good to go with this change. |
|
/uncc |
| approvers: | ||
| - deads2k | ||
| - dgoodwin | ||
| - stbenjam |
There was a problem hiding this comment.
Please feel free to add yourself here, we'll need your expertise to approve having dug into this.
| cpu: 10m | ||
| memory: 10Mi | ||
| documentation: |- | ||
| A observer for watch and record cluster resources |
There was a problem hiding this comment.
| A observer for watch and record cluster resources | |
| An observer to watch all changes to a defined set of cluster resources throughout the life of the cluster, and record them to a git repository. |
| commands: observers-resource-watch-commands.sh | ||
| resources: | ||
| requests: | ||
| cpu: 10m |
There was a problem hiding this comment.
I would bump that up quite a bit, from my testing it looks like we sometimes struggle to commit to the repo fast enough, some of that is probably disk but I think we should err on the side of caution and a tenth of a CPU is very low. (this is 0.01 CPUs) I suggest just changing this to "1" for a full CPU, or maybe just "500m" for a half CPU.
| resources: | ||
| requests: | ||
| cpu: 10m | ||
| memory: 10Mi |
There was a problem hiding this comment.
Seems dangerously low as well, maybe 500Mi?
| kill ${CHILDREN} && wait | ||
| fi | ||
|
|
||
| tar -czC $REPOSITORY_PATH -f "${ARTIFACT_DIR}/resource-watch-store.tar.gz" . |
There was a problem hiding this comment.
Any idea why this is coming out not gzipped? https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/origin-ci-test/pr-logs/pull/openshift_release/33109/rehearse-33109-periodic-ci-openshift-release-master-ci-4.12-e2e-aws-ovn-serial/1583447789566693376/artifacts/e2e-aws-ovn-serial/observers-resource-watch/artifacts/
Is this the thing we've heard mentioned where gzip appears to be getting automatically undone somewhere in our artifacts stack? (cc @DennisPeriquet @stbenjam, can't recall who mentioned this) If so we should probably just skip the gzip step.
I do have one nit here, could we structure this such that there's another sub-directory in play? I notice when you download the tar and extract it, all the files drop in your current dir. It would be great if they were isolated in a sub-dir so we don't accidentally spew the files all over places we didn't intend to.
I think this would be using REPOSITORY_PATH="${ARTIFACT_DIR}/resource-watch-store/repo", but when we go to tar, only using ${ARTIFACT_DIR}/resource-watch-store/
There was a problem hiding this comment.
it looks like it's actually zipped even through it appears with just .tar.
Mozart:Downloads dperique$ file resource-watch-store.tar
resource-watch-store.tar: gzip compressed data
Mozart:Downloads dperique$ ls -lh resource-watch-store.tar
-rw-r--r--@ 1 dperique staff 3.8M Oct 24 08:59 resource-watch-store.tar
Mozart:Downloads dperique$ mv resource-watch-store.tar resource-watch-store.tar.gz
Mozart:Downloads dperique$ gzip -d resource-watch-store.tar.gz
Mozart:Downloads dperique$ ls -lh resource-watch-store.tar
-rw-r--r-- 1 dperique staff 11M Oct 24 08:59 resource-watch-store.tar
| set -o nounset | ||
| set -o pipefail | ||
|
|
||
| export REPOSITORY_PATH="${ARTIFACT_DIR}/resource-watch-store" |
There was a problem hiding this comment.
It's neat that we still get the git repo even if the signal is lost and we fail to tar it up.
xueqzhan
force-pushed
the
resource-watch-observer
branch
from
17a0c28
to
76e38c9
Compare
|
/lgtm |
xueqzhan
force-pushed
the
resource-watch-observer
branch
from
76e38c9
to
905056a
Compare
|
/lgtm |
|
@xueqzhan: The following tests failed, say
Full PR test history. Your PR dashboard. 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. |
|
@jmguzik @bbguimaraes observers have not caused blocking issues so far. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: danilo-gemoli, dgoodwin, xueqzhan 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
bot
added
the
approved
|
@xueqzhan: Updated the
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. |
Hook up resource watch observer
TRT-469