Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update cli-functions.sh #3732

Merged
merged 3 commits into from
Jan 14, 2017
Merged

Update cli-functions.sh #3732

merged 3 commits into from
Jan 14, 2017

Conversation

riuvshin
Copy link
Contributor

@riuvshin riuvshin commented Jan 13, 2017

needed for #3684
remove return statement due to issue where there is no changes to CLI nightly image CLI think that is obsolete but it is actual as image was not changed.

@TylerJewell
Copy link

I think we have some logic that can detect if the pull was successful. I will find that and we will return if you really pull and not return otherwise.

@codenvy-ci
Copy link

Build # 1633 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/1633/ to view the results.

@riuvshin
Copy link
Contributor Author

@TylerJewell @benoitf I think we should not check the date, because date will be always obosolete if we will not have changes to CLI. do you know is it possible to get image ID? from docker hub? what it we will compare local image ID and remote it they are different we do pull. (not sure if that works)

@benoitf
Copy link
Contributor

benoitf commented Jan 14, 2017

@riuvshin unfornatelly the digests are not available on dockerhub. This is why at a time we wanted that ci push that information somewhere each time a nightly was pushed

@riuvshin
Copy link
Contributor Author

@benoitf does digest -eq image id? maybe image id is available..

@benoitf
Copy link
Contributor

benoitf commented Jan 14, 2017

no we don't have it until we pull image locally :-/
with previous v1 registry we had but not with v2

@TylerJewell
Copy link

Why don't we do a docker inspect of the existing image to get the digest ID, then do a pull, and then we do a new docker inspect after the pull to see if the digest has changed. If the digest has not changed, we continue operation. If the digest has changed, then we stop?

@riuvshin
Copy link
Contributor Author

@TylerJewell that could work, or we can
1 get current image id
2 do pull
3 compare image id with id after pull,
3a if they are the same we proceed
3b if they are different we exit CLI and ask to rerun CLI

lol it seems it is exactly the same :D

@TylerJewell
Copy link

I have a branch that is working with the digest comparison. Let me push it up and see what people think. I think that the digest check is a more reliable version for us to use.

@TylerJewell
Copy link

Ok - so I have updated the branch with a different form of check.

  1. Capture the existing images's digest with docker images -q --no-trunc --digests
  2. Pull the image if --fast is not provided.
  3. Capture the new images digest
  4. And then compare - if the digests are different, stop the CLI and ask for a rerun

@codenvy-ci
Copy link

@benoitf
Copy link
Contributor

benoitf commented Jan 14, 2017

always pulling by default for nightly seems to be the best option.
Maybe we could store in /data folder the date of the check and then not pull every time we launch the command.

@TylerJewell
Copy link

Happy to add that improvement if you don't think the --fast optimization is enough.

I can only place that check into directories that have been initialized so it is not universal. I like just having the fast option.

@benoitf
Copy link
Contributor

benoitf commented Jan 14, 2017

using fast option is OK for now we can improve later based on users feedback

@TylerJewell
Copy link

Let's get @riuvshin input and if he is ok will merge.

@riuvshin
Copy link
Contributor Author

+1 (i cant approve this )

@riuvshin riuvshin merged commit 2eed6e1 into master Jan 14, 2017
@TylerJewell TylerJewell deleted the riuvshin-patch-2 branch January 16, 2017 14:57
@slemeur slemeur added this to the 5.1.0 milestone Jan 17, 2017
@slemeur slemeur added the kind/enhancement A feature request - must adhere to the feature request template. label Jan 17, 2017
@JamesDrummond JamesDrummond mentioned this pull request Jan 31, 2017
9 tasks
JPinkney pushed a commit to JPinkney/che that referenced this pull request Aug 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement A feature request - must adhere to the feature request template.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants