Add GUBERNATOR flag which produces g8r link for node e2e tests #30414
Conversation
mnshaw
force-pushed
the
run-remote-tests
branch
from
48719b5 to
659f64a
Compare
k8s-github-robot
added
size/L
| --hosts="$hosts" --images="$images" --cleanup="$cleanup" \ | ||
| --results-dir="$artifacts" --ginkgo-flags="$ginkgoflags" \ | ||
| --image-project="$image_project" --instance-name-prefix="$instance_prefix" --setup-node="true" \ | ||
| --delete-instances="$delete_instances" --test_args="$test_args" --instance-metadata="$metadata" |
There was a problem hiding this comment.
Is there any difference between this command and the one below? If not, move it out of the conditional. If there is, can you extract the difference to a variable that gets added in?
There was a problem hiding this comment.
Ah, I see the difference is the presence of | tee build-log.txt. As long as that goes to the artifacts directory, I think it's safe to always save it.
|
Looks good, mostly code hygiene comments around the script. |
| bucket_name="${USER}-g8r-logs" | ||
| echo "" | ||
| echo "Using bucket ${bucket_name}" | ||
| gsutil mb gs://${bucket_name}/ |
There was a problem hiding this comment.
I think it would be good to put some checks at the beginning of the script that:
- User has gsutil
- gcloud auth is set up correctly
Also, I think it would be good to warn the user that this script will upload the build logs (located at X) to a public bucket, and get their consent. You could add an environment variable to skip this step for experienced users.
|
Still need to:
Known issue: The timestamps seem to be in a different timezone |
mnshaw
force-pushed
the
run-remote-tests
branch
2 times, most recently
from
eaafdbe to
dd00779
Compare
mnshaw
changed the title
|
|
||
|
|
||
| if [[ $# -eq 0 || ! $1 =~ ^[Yy]$ ]]; then | ||
| read -p "Do you want to run gubernator.sh and upload logs to GCS? [y/n]" yn |
|
LGTM, after you fix this: |
timstclair
added
release-note-none
mnshaw
changed the title
mnshaw
force-pushed
the
run-remote-tests
branch
2 times, most recently
from
5126306 to
a9cf727
Compare
|
LGTM. Please squash the commits. You can ignore the node failure. |
mnshaw
force-pushed
the
run-remote-tests
branch
from
a9cf727 to
d69252c
Compare
|
GCE e2e build/test passed for commit d69252c. |
timstclair
added
the
lgtm
dchen1107
added
the
priority/critical-urgent
|
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
|
GCE e2e build/test passed for commit d69252c. |
|
Automatic merge from submit-queue |
Automatic merge from submit-queue Gubernator bug fixes: mv and GCS bucket permissions Fixed issue where results file was not moved correctly, and also the permissions issue with the GCS bucket. Will rebase after kubernetes#30414 is merged @timstclair
When you run 'make tests-e2e-node REMOTE=true GUBERNATOR=true' outputs a URL to view the test results on Gubernator.
Should work after my PR for Gubernator is merged.@timstclair
This change is