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

feature: Use image digest in csv #274

Merged
merged 7 commits into from
Jul 8, 2020
Merged

Conversation

AndrienkoAleksandr
Copy link
Contributor

@AndrienkoAleksandr AndrienkoAleksandr commented May 24, 2020

@AndrienkoAleksandr AndrienkoAleksandr changed the title Use digest in csv feature: Use image digest in csv May 24, 2020
olm/addDigests.sh Outdated Show resolved Hide resolved
olm/addDigests.sh Outdated Show resolved Hide resolved
@nickboldt
Copy link
Contributor

Can you also rename variables so that we're doing the work requested in eclipse-che/che#16815 ?

@AndrienkoAleksandr
Copy link
Contributor Author

Can you also rename variables so that we're doing the work requested in eclipse-che/che#16815

Most envs has prefix RELATED_IMAGES_
https://gist.github.com/AndrienkoAleksandr/02dd792d63878ae5516ec9069950839e#file-gistfile1-txt-L492 ,
But, yes, for default images, I didn't change name https://gist.github.com/AndrienkoAleksandr/02dd792d63878ae5516ec9069950839e#file-gistfile1-txt-L436 ...

@tolusha What Do you think could we rename default images env variable, or we can get issue with back compatibility?

@tolusha
Copy link
Contributor

tolusha commented May 26, 2020

@AndrienkoAleksandr
We shouldn't fail into any issue with that.

@tolusha
Copy link
Contributor

tolusha commented Jun 3, 2020

@AndrienkoAleksandr
When you rebase on master update this line pls
https://github.com/eclipse/che-operator/blob/master/olm/update-nightly-olm-files.sh#L74

@AndrienkoAleksandr
Copy link
Contributor Author

AndrienkoAleksandr commented Jun 10, 2020

I updated pull request description.

@tolusha
Copy link
Contributor

tolusha commented Jun 10, 2020

  relatedImages:
    - name: che-operator-digest
      image: docker.io/aandrienko/che-operator@sha256:7f4a80ab817caa97ca808d8934e542fceedeeed9468f13babb6a3a1fb066ac30
      # tag: docker.io/aandrienko/che-operator:digest
    - name: che-server-7.14.1

Maybe it isn't relevant but tag is digest
# tag: docker.io/aandrienko/che-operator:digest

@AndrienkoAleksandr
Copy link
Contributor Author

Maybe it isn't relevant but tag is digest

Yes, I really used such name for my test tag.

@che-bot
Copy link
Contributor

che-bot commented Jun 10, 2020

Latest version of Eclipse installed and tested successfully on minikube.

@che-bot
Copy link
Contributor

che-bot commented Jun 12, 2020

Latest version of Eclipse installed and tested successfully on minikube.

@che-bot
Copy link
Contributor

che-bot commented Jun 12, 2020

Latest version of Eclipse installed and tested successfully on minikube.

@che-bot
Copy link
Contributor

che-bot commented Jun 15, 2020

Latest version of Eclipse installed and tested successfully on minikube.

@che-bot
Copy link
Contributor

che-bot commented Jun 15, 2020

Latest version of Eclipse installed and tested successfully on minikube.

@nickboldt nickboldt self-requested a review June 30, 2020 14:03
Copy link
Contributor

@nickboldt nickboldt left a comment

Choose a reason for hiding this comment

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

I'm confident that these changes will break CRW 2.3 integration via the transformation scripts we use to turn che-operator into crw-operator, but I'm also confident that we can adapt to your changes.

PR related to downstream: https://github.com/redhat-developer/codeready-workspaces-operator/pull/25/files

@AndrienkoAleksandr
Copy link
Contributor Author

I will generate nightly scripts later, otherwise this pr is always in merge conflict.

@nickboldt nickboldt self-requested a review July 6, 2020 20:09
Copy link
Contributor

@nickboldt nickboldt left a comment

Choose a reason for hiding this comment

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

Now that this is rebased (after the last 7.15.x release) it can be merged.

Also thanks for https://github.com/redhat-developer/codeready-workspaces-operator/pull/25/files

+1, approved.

Downstream issue: https://issues.redhat.com/browse/CRW-1026

@tolusha
Copy link
Contributor

tolusha commented Jul 7, 2020

@nickboldt
Could you clarify why we have to wait for the last 7.15.x release?

@nickboldt
Copy link
Contributor

nickboldt commented Jul 7, 2020

I was saying that now that it's updated to the last 7.15.x release, it's safe to merge. the new CSVs are no longer causing a conflict. Merge when ready, I already +1'd above.

I'll let you decide if the failing PR checks matter. :)

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
…. Fix some shellchecks.

Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
Signed-off-by: Oleksandr Andriienko <oandriie@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Jul 7, 2020

Latest version of Eclipse Che installed and tested successfully on minikube.

@AndrienkoAleksandr
Copy link
Contributor Author

CI is green

@AndrienkoAleksandr AndrienkoAleksandr merged commit adfe698 into master Jul 8, 2020
@AndrienkoAleksandr AndrienkoAleksandr deleted the useDigestInCSV branch July 8, 2020 07:43
@nickboldt
Copy link
Contributor

This change seems to have caused eclipse-che/che#17480 but I'm not sure if the formatting change is a problem or just results in illegible yaml.

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.

4 participants