Avoid redundant copying of tars during kube-up for gce if the same file already exists #46792
Conversation
…ady exists in gce
k8s-ci-robot
added
the
cncf-cla: yes
|
Hi @ianchakeres. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with I understand the commands that are listed here. 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. |
k8s-ci-robot
added
the
needs-ok-to-test
|
@pwittrock - please take a quick peak at this PR and let me know if you recommend any changes. |
k8s-github-robot
added
size/S
|
/sig cluster-lifecycle |
k8s-ci-robot
added
the
sig/cluster-lifecycle
|
@k8s-bot ok to test |
k8s-ci-robot
removed
the
needs-ok-to-test
|
@ianchakeres: you can't request testing unless you are a kubernetes member. 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. |
|
@k8s-bot ok to test |
|
@ianchakeres Please add a release note in the form show in the template: e.g. |
cluster/gce/util.sh
Outdated
| if [[ -n ${remote_tar_hash} ]]; then | ||
| local -r local_tar_hash=$(gsutil hash -h -m ${tar} 2>/dev/null | grep "Hash (md5):" | awk -F ':' '{print $2}') | ||
| if [[ "${remote_tar_hash}" == "${local_tar_hash}" ]]; then | ||
| echo "+++ ${basename_tar} uploaded earlier and hash matches" |
There was a problem hiding this comment.
Maybe output the hash value as well
There was a problem hiding this comment.
Sounds good. Working on an update now.
cluster/gce/util.sh
Outdated
| #if it matches, then don't bother uploading it again | ||
| local -r remote_tar_hash=$(gsutil hash -h -m ${staging_path}/${basename_tar} 2>/dev/null | grep "Hash (md5):" | awk -F ':' '{print $2}') | ||
| if [[ -n ${remote_tar_hash} ]]; then | ||
| local -r local_tar_hash=$(gsutil hash -h -m ${tar} 2>/dev/null | grep "Hash (md5):" | awk -F ':' '{print $2}') |
There was a problem hiding this comment.
this is fine, but would be a bit cleaner if pulled into a function to return the hash for a given name
There was a problem hiding this comment.
Ok. I'll update the next revision to use a separate function for calling gsutil to get the hash and parse the output.
cluster/gce/util.sh
Outdated
|
|
||
| #check whether this tar alread exists and has the same hash | ||
| #if it matches, then don't bother uploading it again | ||
| local -r remote_tar_hash=$(gsutil hash -h -m ${staging_path}/${basename_tar} 2>/dev/null | grep "Hash (md5):" | awk -F ':' '{print $2}') |
There was a problem hiding this comment.
How does this work against a remote tar? I am not familiar with gsutil but the description is The hash command calculates hashes on a local file. Is this working on a remote file or a local file?
There was a problem hiding this comment.
In this particular case the hash is operating on the remote file, and I tested that it works.
Looks like that documentation you referenced is out of date, and this feature was added to support hash on cloud files in gsutil v4.21. Here is the related issue for tracking hash of cloud files - GoogleCloudPlatform/gsutil#369
|
/assign |
k8s-github-robot
added
release-note
k8s-github-robot
added
size/M
|
@k8s-bot pull-kubernetes-e2e-gce-etcd3 test this |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
@vishh Would you take a look at this since it looks like you can approve? |
|
@k8s-bot pull-kubernetes-e2e-gce-etcd3 test this |
|
Yesterday I encountered some test flakes inside e2e-gce-etcd3 on this PR #46792. I think they relate to these two reported flaky tests issues #43520 & #47446. From my investigation the failures mention timeouts. Here are the the failed test runs' output: |
|
@vishh can you please take a quick look and approve this PR? Change is about ~25 lines and half are clarifying comments. If you have any questions or comments, just let me know. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ianchakeres, pwittrock, vishh Associated issue: 46791 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
k8s-github-robot
added
the
approved
| # Copy a release tar and its accompanying hash. | ||
| function copy-to-staging() { | ||
| local -r staging_path=$1 | ||
| local -r gs_url=$2 | ||
| local -r tar=$3 | ||
| local -r hash=$4 | ||
| local -r basename_tar=$(basename ${tar}) | ||
|
|
||
| #check whether this tar alread exists and has the same hash |
|
/test pull-kubernetes-e2e-gce-etcd3 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ianchakeres, pwittrock, vishh Associated issue: 46791 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
|
Automatic merge from submit-queue (batch tested with PRs 47403, 46646, 46906, 46527, 46792) |
What this PR does / why we need it:
Whenever I execute cluster/kube-up.sh it copies my tar files to google cloud, even if the files haven't changed. This PR checks to see whether the files already exist, and avoids uploading them again. These files are large and can take a long time to upload.
Which issue this PR fixes: fixes #46791
Special notes for your reviewer:
Here is the new output:
cluster/kube-up.sh
... Starting cluster in us-central1-b using provider gce
... calling verify-prereqs
... calling verify-kube-binaries
... calling kube-up
Project: PROJECT
Zone: us-central1-b
+++ Staging server tars to Google Storage: gs://kubernetes-staging-PROJECT/kubernetes-devel
+++ kubernetes-server-linux-amd64.tar.gz uploaded earlier, cloud and local file md5 match (md5 = 3a095kcf27267a71fe58f91f89fab1bc)
Release note:
cluster/kube-up.sh on gce now avoids redundant copying of kubernetes tars if the local and cloud files' md5 hash match