gcp: adding image check before creating image #589
Conversation
|
@bpradipt @littlejawa, yesterday I force pushed this commit to the PR branch, I remember seeing that commit on that PR, for some reason, it looks like it got lost, maybe with another force-push. So my bad. |
beraldoleal
force-pushed
the
gcp-image-check
branch
from
ce9465b to
a765819
Compare
As following other providers logic. Signed-off-by: Beraldo Leal <[email protected]>
beraldoleal
force-pushed
the
gcp-image-check
branch
from
a765819 to
7c3bef4
Compare
|
@beraldoleal: all tests passed! 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-sigs/prow repository. I understand the commands that are listed here. |
|
|
||
| # This command returns something like this: | ||
| # https://www.googleapis.com/compute/v1/projects/my-project/global/images/my-image | ||
| # Use `cut` to the image name |
There was a problem hiding this comment.
It is obvious that cut is used below, it is less obvious what we want to drop.
| # Use `cut` to the image name | |
| # we want to drop the leading "https://www.googleapis.com/compute/v1/" |
| --project="${GCP_PROJECT_ID}" \ | ||
| --format='get(selfLink)' 2>/dev/null | cut -d/ -f6-) | ||
|
|
||
| if [[ $? -ne 0 ]]; then |
There was a problem hiding this comment.
AFAICT this branch is dead code.
greg@bahia:~$ image_path=$(false | cut -d/ -f6-)
greg@bahia:~$ echo $?
0
| else | ||
| echo "Image mismatch: GCP image (${image_path}) different from ConfigMap (${latest_image_id})." | ||
| return 2 | ||
| fi |
There was a problem hiding this comment.
This image_exists function turns out to have non-trivial semantics, not just returning true or false. IMO it would be clearer to open-code it in its unique caller. This would spare the need for the exit status values.
There was a problem hiding this comment.
gcp: adding image check before creating image
As following other providers logic.Signed-off-by: Beraldo Leal [email protected]
The commit message would be a good place to describe the various cases that need to be handled BTW.
In the meantime, I went to #584 and I realized that you borrowed the convoluted image_exists pattern from there and it is already merged 😞 ... well, I guess I won't fight if everyone is apparently happy with that.
As following other providers logic. A follow up to #584.