chore: update YQ everywhere and deploy-renku wants split values files. #60
Conversation
There was a problem hiding this comment.
Thanks for doing this @aledegano! Two things:
- could you please make a test PR that uses this branch in the main renku repo to verify it all works as expected?
- can you remove the
xinset -exin all the scripts? I know you didn't add it, but since we're updating things it seems like a good opportunity
aledegano
force-pushed
the
split_values_file
branch
from
2593f0c to
290e662
Compare
This is the sister PR: SwissDataScienceCenter/renku#3191 in Renku that uses the new I am not sure if I can "test" the other actions as they are invoked in very specific situations. I have tested in my terminal all the
Good point, done. |
aledegano
force-pushed
the
split_values_file
branch
from
290e662 to
33f9fc2
Compare
yes, that's always problematic with these - the only thing that raises orange flags is the change of chartpress version. This has caused issues in the past and there was a reason why it was pinned. I believe this issue may be a part of it? But for sure it changes the tag format. It could be that more recent versions do it better, but 1.0 was definitely a problem in the past with our workflows. |
Okay I'll double check it then. |
aledegano
force-pushed
the
split_values_file
branch
from
33f9fc2 to
63d4ef0
Compare
aledegano
force-pushed
the
split_values_file
branch
from
63d4ef0 to
c941934
Compare
- deploy-renku now expects two files for the values of the Helm chart to deploy: one is the secrets that are stored in GH organization, the other is a file passed to the action, meant to be in the Renku repository itself. - YQ has been updated to version 4.x everywhere and the commands fixed according to the need of the new version - The base Docker image have also been updated - YQ is now installed in all actions through the Alpine package directly
aledegano
force-pushed
the
split_values_file
branch
from
c941934 to
6742941
Compare
| APP_ID=$(echo $gitlab_app | jq -r '.application_id') | ||
| APP_SECRET=$(echo $gitlab_app | jq -r '.secret') | ||
| export APP_ID=$(echo $gitlab_app | jq -r '.application_id') | ||
| export APP_SECRET=$(echo $gitlab_app | jq -r '.secret') |
There was a problem hiding this comment.
I am pretty sure we don't need the export here and in front of APP_ID and for RENKU_VALUES_FILE. The variables will be available within the script even without export. So no need to do it.
| export RENKU_VALUES_FILE="/renku-values.yaml" | ||
|
|
||
| # merge the secret and clear value files | ||
| printf "%s" "$RENKU_SECRET_VALUES" > $RENKU_VALUES_FILE |
There was a problem hiding this comment.
We need to update the README.md for these changes accordingly. According to the README the secret values are called just RENKU_VALUES.
| @@ -12,8 +12,14 @@ echo $DOCKER_PASSWORD | docker login -u $DOCKER_USERNAME --password-stdin | |||
| # set up kube context and values file | |||
| echo "$RENKUBOT_KUBECONFIG" > "$KUBECONFIG" && chmod 400 "$KUBECONFIG" | |||
|
|
|||
| # set up the values file | |||
| printf "%s" "$RENKU_VALUES" | sed "s/<replace>/${RENKU_RELEASE}/" > $RENKU_VALUES_FILE | |||
| export RENKU_VALUES_FILE="/renku-values.yaml" | |||
There was a problem hiding this comment.
There is already a RENKU_VALUES_FILE variable defined (or at least that is what the README says) and that contains the values.yaml file that comes with the helm chart. But here this is something else I think? Is this supposed to contain the non-secret CI values?
|
|
||
| # merge the secret and clear value files | ||
| printf "%s" "$RENKU_SECRET_VALUES" > $RENKU_VALUES_FILE | ||
| yq eval-all --inplace '. as $item ireduce ({}; . * $item )' $RENKU_VALUES_FILE $RENKU_CLEAR_VALUES_FILE |
There was a problem hiding this comment.
I think it is better to let helm take care of this if we simply pass the files on to the helm command in the python script that does helm upgrade --install. I.e. we can do helm install -f non-secret.values.yaml -f secret-values.yaml --set extra-values-from-comment. Then we dont have to do this yq magic stuff. Helm already handles this.
|
I tested this on the notebooks repo and it does not work: The problem is that the deploy actions looks for the values file locally. It would be nice if we do not have to have copies of the clear values file in every component repo. This does not make a lot of sense since now we have one helm chart in the renku repo. We should address this as well. |
deploy-renku now expects two files for the values of the Helm chart to deploy:
one is the secrets that are stored in GH organization, the other is a file passed
to the action, meant to be in the Renku repository itself.
YQ has been updated to version 4.x everywhere and the commands fixed according
to the need of the new version
The base Docker image have also been updated
YQ is now installed in all actions through the Alpine package directly