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

fix: better recovery from a failed skopeo... #529

Merged
merged 2 commits into from Dec 2, 2021

Conversation

nickboldt
Copy link
Contributor

@nickboldt nickboldt commented Dec 1, 2021

What does this PR do?

fix: better recovery from a failed skopeo inspect -- try again and if not resolved after 2 attempts, DON'T CHANGE the digest to a nullstring

Change-Id: Ic65d99256542b646eb7f3e64f404f40071edab72
Signed-off-by: nickboldt nboldt@redhat.com

Screenshot/screencast of this PR

N/A

What issues does this PR fix or reference?

Avoids this problem:
https://github.com/eclipse-che/che-devfile-registry/pull/526/files#diff-d567d4dc0d879e5b1da3a21d592817871f3651f034a8d3c5d444a18a71ba243eR12

How to test this PR?

  1. Edit this file: che-devfiles-registry/dockerfiles/antora-2.3/Dockerfile
  2. set an invalid BASE_IMAGE address like ARG BASE_IMAGE="docker.io/antora/antFFora:2.3.3"
  3. run ./build/workflows/check-sidecar-image-digests.sh

After 30 seconds, no change will be applied to the registry and the processing of other Dockerfiles will continue. \o/

To prove that it WILL change when it DOES find a different latest digest:

  1. Edit this file: che-devfiles-registry/dockerfiles/antora-2.3/Dockerfile
  2. set a different SHA, like FROM docker.io/antora/antora@sha256:f00barf
  3. run ./build/workflows/check-sidecar-image-digests.sh

Watch as the file is changed and your bad digest is reverted to a valid one. \o/

If you let the script run its course you should see ONE file changed: dockerfiles/golang-1.17/Dockerfile

https://github.com/eclipse-che/che-devfile-registry/pull/526/files#diff-8f51fac72ac1fc135355f5eaabe8e7984b6f79048c983e55dd78ca94d040677dR12

PR Checklist

As the author of this Pull Request I made sure that:

Reviewers

Reviewers, please comment how you tested the PR when approving it.

… not resolved after 2 attempts, DON'T CHANGE the digest to a nullstring

Change-Id: Ic65d99256542b646eb7f3e64f404f40071edab72
Signed-off-by: nickboldt <nboldt@redhat.com>
@github-actions
Copy link

github-actions bot commented Dec 1, 2021

Click here to review and test in web IDE: Contribute

…close #526

Change-Id: I11a9933f515fcd7a9fafa3c62608b771d92e50ef
Signed-off-by: nickboldt <nboldt@redhat.com>
@github-actions
Copy link

github-actions bot commented Dec 1, 2021

Click here to review and test in web IDE: Contribute

@nickboldt
Copy link
Contributor Author

Replaces #526

@nickboldt
Copy link
Contributor Author

nickboldt commented Dec 1, 2021

@nickboldt
Copy link
Contributor Author

If you approve this, please merge it. The question around node10/12/14/16 can be handled in a separate issue, should we agree it's time to update to the latest TS lang server.

Copy link
Contributor

@svor svor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested as described in the description, it works as expected. Thanks!

@svor svor merged commit 5ede1d0 into main Dec 2, 2021
@svor svor deleted the better-recovery-from-skopeo-query-fail branch December 2, 2021 09:56
@che-bot che-bot added this to the 7.41 milestone Dec 2, 2021
@svor
Copy link
Contributor

svor commented Dec 2, 2021

Side note: can we get rid of node 10 and move to node 12 or 14? Or even node 16?

https://catalog.redhat.com/software/containers/ubi8/nodejs-16/615aee9fc739c0a4123a87e1

Does the lang server we use in CRW (and presumably Che too) work with newer node, or is it tied to 10?

We're on typescript lang server 0.3.7 - https://github.com/redhat-developer/codeready-workspaces-images/blob/crw-2-rhel-8/codeready-workspaces-plugin-java8/build/build_node10.sh#L19 Latest is 0.8.1 - https://github.com/typescript-language-server/typescript-language-server/releases/tag/v0.8.1

We're on typescript 3.4.5: https://github.com/redhat-developer/codeready-workspaces-images/blob/crw-2-rhel-8/codeready-workspaces-plugin-java8/build/build_node10.sh#L18 Latest is 4.5.2: https://www.npmjs.com/package/typescript

We can try to update node, for now I don't know any problems with it, but it needs to be tested with all samples which required node.
Also i'm not sure why do we need typescript lang server and typescript in the image, i don't see that these packages are installed in upstream

@svor
Copy link
Contributor

svor commented Dec 2, 2021

@nickboldt I've created an issue for updating Node base image in CRW https://issues.redhat.com/browse/CRW-2527

svor pushed a commit that referenced this pull request Dec 3, 2021
* fix: output of meta.yaml should be yaml content

Change-Id: Idcbe30c9f1c8ee1f0b0162a3061ee9210dd4d4cb
Signed-off-by: Florent Benoit <fbenoit@redhat.com>

* [release] Bump to 7.41.0-SNAPSHOT in main (#528)

Signed-off-by: Mykhailo Kuznietsov <mkuznets@redhat.com>

Co-authored-by: Mykhailo Kuznietsov <mkuznets@redhat.com>

* chore(digests): update dockerfile base images

Signed-off-by: Mykhailo Kuznietsov <mkuznets@redhat.com>

* chore(sidecars): bump to new sidecar tags

Signed-off-by: Mykhailo Kuznietsov <mkuznets@redhat.com>

* fix: better recovery from a failed skopeo... (#529)

* fix: better recovery from a failed skopeo inspect -- try again and if not resolved after 2 attempts, DON'T CHANGE the digest to a nullstring

Change-Id: Ic65d99256542b646eb7f3e64f404f40071edab72
Signed-off-by: nickboldt <nboldt@redhat.com>

* chore: include newer digest in this PR so we can simply merge this and close #526

Change-Id: I11a9933f515fcd7a9fafa3c62608b771d92e50ef
Signed-off-by: nickboldt <nboldt@redhat.com>

* [release] Bump to 7.40.0 in 7.40.x

Signed-off-by: Mykhailo Kuznietsov <mkuznets@redhat.com>

Co-authored-by: Florent Benoit <fbenoit@redhat.com>
Co-authored-by: Mykhailo Kuznietsov <mkuznets@redhat.com>
Co-authored-by: Nick Boldt <nboldt@redhat.com>
svor pushed a commit that referenced this pull request Dec 4, 2021
* fix: output of meta.yaml should be yaml content

Change-Id: Idcbe30c9f1c8ee1f0b0162a3061ee9210dd4d4cb
Signed-off-by: Florent Benoit <fbenoit@redhat.com>

* [release] Bump to 7.41.0-SNAPSHOT in main (#528)

Signed-off-by: Mykhailo Kuznietsov <mkuznets@redhat.com>

Co-authored-by: Mykhailo Kuznietsov <mkuznets@redhat.com>

* chore(digests): update dockerfile base images

Signed-off-by: Mykhailo Kuznietsov <mkuznets@redhat.com>

* chore(sidecars): bump to new sidecar tags

Signed-off-by: Mykhailo Kuznietsov <mkuznets@redhat.com>

* fix: better recovery from a failed skopeo... (#529)

* fix: better recovery from a failed skopeo inspect -- try again and if not resolved after 2 attempts, DON'T CHANGE the digest to a nullstring

Change-Id: Ic65d99256542b646eb7f3e64f404f40071edab72
Signed-off-by: nickboldt <nboldt@redhat.com>

* chore: include newer digest in this PR so we can simply merge this and close #526

Change-Id: I11a9933f515fcd7a9fafa3c62608b771d92e50ef
Signed-off-by: nickboldt <nboldt@redhat.com>

* fix: #20880 instead of a separate stage for dwtemplates, just use the node16 image as the builder, and then we don't have to repeat the same several steps in a new container... and don't have to fix perms in /devfiles/ folder either (#534)

Change-Id: Ib61cfe1548e73ed2a8225596088250248b907c50
Signed-off-by: nickboldt <nboldt@redhat.com>

* [release] Bump to 7.40.0 in 7.40.x

Signed-off-by: Mykhailo Kuznietsov <mkuznets@redhat.com>

Co-authored-by: Florent Benoit <fbenoit@redhat.com>
Co-authored-by: Mykhailo Kuznietsov <mkuznets@redhat.com>
Co-authored-by: Nick Boldt <nboldt@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants