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

feat: Remove image refs in defaults.go and replace by env vars #172

Merged
merged 16 commits into from
Feb 18, 2020

Conversation

AndrienkoAleksandr
Copy link
Contributor

@AndrienkoAleksandr AndrienkoAleksandr commented Feb 14, 2020

Remove image refs in defaults.go and replace by env vars

Reference issue

eclipse-che/che#15874

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>
…cheVersion.

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>
release-operator-code.sh Outdated Show resolved Hide resolved
pkg/controller/che/che_controller.go Outdated Show resolved Hide resolved
@davidfestal
Copy link
Contributor

Just had 2 comments when reviewing, but great work @AndrienkoAleksandr !

Signed-off-by: Anatoliy Bazko <abazko@redhat.com>
Signed-off-by: Anatoliy Bazko <abazko@redhat.com>
Signed-off-by: Anatoliy Bazko <abazko@redhat.com>
@tolusha tolusha changed the title WIP Remove image refs in defaults.go and replace by env vars feat: Remove image refs in defaults.go and replace by env vars Feb 15, 2020
@tolusha tolusha marked this pull request as ready for review February 15, 2020 15:17
@nickboldt
Copy link
Contributor

Is this planned for inclusion in 7.9?

@tolusha
Copy link
Contributor

tolusha commented Feb 15, 2020

@nickboldt
Yes

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

tolusha commented Feb 17, 2020

[test]

@flacatus
Copy link
Contributor

[test]

flacatus and others added 2 commits February 17, 2020 14:00
Signed-off-by: flacatus <flacatus@redhat.com>
Add docker images env to operator-local.yaml
Copy link
Contributor

@davidfestal davidfestal left a comment

Choose a reason for hiding this comment

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

Great work ! Thanks !

@nickboldt
Copy link
Contributor

Looking forward to this feature being in Che and CRW.

Any advice for downstream implementation welcome, especially for files that have to be synced as is or modified for downstream.

https://github.com/redhat-developer/codeready-workspaces-operator/blob/master/operator.Jenkinsfile

@davidfestal
Copy link
Contributor

davidfestal commented Feb 17, 2020

So one last thing we seems to have forgotten: We should update the OLM packages in order to integrate the changes in the operator.yaml.
@tolusha @AndrienkoAleksandr

- name: "IMAGE_default_keycloak"
value: "quay.io/eclipse/che-keycloak:7.8.0"
- name: "IMAGE_default_che_workspace_plugin_broker_metadata"
value: "quay.io/crw/pluginbroker-metadata-rhel8:2.1"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm... Guys, I see that our release scripts replace this images from crw to the che upstream:

quay.io/crw/pluginbroker-metadata-rhel8:2.1 -> quay.io/eclipse/che-plugin-metadata-broker:v3.1.0

Maybe Should we do the same in this pr? Or Does our scripts mistake?

@AndrienkoAleksandr
Copy link
Contributor Author

So one last thing we seems to have forgotten: We should update the OLM packages in order to integrate the changes in the operator.yaml.
@tolusha @AndrienkoAleksandr

I think we can do that even after merge.

@davidfestal
Copy link
Contributor

So one last thing we seems to have forgotten: We should update the OLM packages in order to integrate the changes in the operator.yaml.
@tolusha @AndrienkoAleksandr

I think we can do that even after merge.

well, that's not ideal. In any case we cannot afford merging this PR and release 7.9.0 while omitting the corresponding OLM change. That would break the next OLM version afaict

* Fix wrong docker images values

The docker images set in this GH repo should be the upstream ones,
not the CRW ones

Signed-off-by: David Festal <dfestal@redhat.com>
@AndrienkoAleksandr AndrienkoAleksandr merged commit e9c5d3f into master Feb 18, 2020
@AndrienkoAleksandr AndrienkoAleksandr deleted the CHE-15874 branch February 18, 2020 08:17
@AndrienkoAleksandr
Copy link
Contributor Author

@davidfestal , @flacatus @tolusha Thank a lot for help and collaboration!

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.

5 participants