Added ability to deploy che plugin registry with ocp.sh #10954
Conversation
skabashnyuk
requested review from
garagatyi,
l0rd,
riuvshin and
sleshchenko
as code owners
|
ci-test |
deploy/openshift/ocp.sh
Outdated
| -p IMAGE_TAG=${CHE_PLUGIN_REGISTRY_IMAGE_TAG} | ||
| CHE_PLUGIN_REGISTRY_ROUTE=$($OC_BINARY get route/che-plugin-registry --namespace=${CHE_OPENSHIFT_PROJECT} -o=jsonpath={'.spec.host'}) | ||
| echo "Che plugin registry deployment complete. $CHE_PLUGIN_REGISTRY_ROUTE" | ||
| ${OC_BINARY} set env dc/che CHE_PLUGIN_REGISTRY_URL="http://$CHE_PLUGIN_REGISTRY_ROUTE/plugins/" |
There was a problem hiding this comment.
As far as I understand it will invoke che deploy one more time. So, user has to wait until Che becomes ready one more time. Can we avoid this? Like deploy the registry and only then deploy Che with right env variable there?
There was a problem hiding this comment.
I think we can avoid it. Not sure about do we need to. @garagatyi wdyt?
There was a problem hiding this comment.
I completely agree with @sleshchenko!
We can add a variable to Che deployment and set it to the correct value in case registry is needed.
There was a problem hiding this comment.
I'll take a look.
There was a problem hiding this comment.
@sleshchenko @garagatyi I've tried to do what you want. But honestly, it required some complex refactoring of ocp.sh and che_deploy.sh that will be thrown away when plugin-registry become a part of che. do you think it's mandatory for this pr?
There was a problem hiding this comment.
Is not exporting of CHE_PLUGIN_REGISTRY_URL enough since deploy script automatically detect envs vars beginning with "CHE_" and passes them to Che deployments?
There was a problem hiding this comment.
we can do that way, but I would say it's a workaround and prefer to do that over the template. Another issue is that registry deployment requires che to be already deployed. To change that I need to rewrite a lot in che_deploy.sh
There was a problem hiding this comment.
@sleshchenko I'll try to test your idea.
| spec: | ||
| containers: | ||
| - image: ${IMAGE}:${IMAGE_TAG} | ||
| imagePullPolicy: Always |
There was a problem hiding this comment.
It may be useful to have an ability to configure this value, for example for testing purpose
| displayName: Memory Limit | ||
| name: MEMORY_LIMIT | ||
| value: 256Mi | ||
|
|
There was a problem hiding this comment.
You can remove extra line here
| name: che-plugin-registry | ||
| parameters: | ||
| - name: IMAGE | ||
| value: eclipse/che-plugin-registry |
There was a problem hiding this comment.
Please consider adding descriptions for these parameters with described default values there.
benoitf
added
status/code-review
|
ci-test |
|
ci-test build report: |
|
@sleshchenko @garagatyi @eivantsov I've made some adjustments in scripts. Now it deploys registry only with che. Please take a look. |
|
ci-test |
deploy/openshift/ocp.sh
Outdated
| @@ -394,3 +400,4 @@ init | |||
| get_tools | |||
| parse_args "$@" | |||
| deploy_che_to_ocp | |||
|
|
|||
…y_che_plugin_registry
|
ci-test build report: |
benoitf
removed
the
status/code-review
What does this PR do?
./ocp.sh --deploy-che --deploy-che-plugin-registryCHE_PLUGIN_REGISTRY_URLfor che-master with a link to che plugin registryWhat issues does this PR fix or reference?
#10842
Release Notes
Added ability to deploy che plugin registry with ocp.sh
Docs PR
n/a