Chart version bump fix #726

RafalKorepta · 2023-09-15T11:52:18Z

Align all appVersion string with artifacthub.io/images annotation.

Fix operator values.yaml to work with bump_chart_version.sh script.
Currently nightly build is failing with the following error:

2023/09/15 01:26:17 Error querying repository tags: unable to open repository 'docker.redpanda.com/redpandadata/redpanda-operator
docker.redpanda.com/redpandadata/configurator': invalid reference: invalid repository

The values.yaml file for operator has 2 repository values which confuses bump_chart_version.sh script.

The docker-tag-list program receives in the REPO variable the following content:

docker.redpanda.com/redpandadata/redpanda-operator
docker.redpanda.com/redpandadata/configurator

This change overcome this issue.

P.S. New version of the Operator will not use configurator container.

REF
https://github.com/redpanda-data/helm-charts/actions/runs/6192749166/job/16813278284#step:6:7

Make new replacement of any occurrence of old version into new version in Chart.yaml file.

alejandroEsc

A few questions, not big.

alejandroEsc · 2023-09-15T16:23:09Z

charts/operator/Chart.yaml

@@ -34,9 +34,9 @@ annotations:
      url: https://helm.sh/docs/intro/install/
  artifacthub.io/images: |
    - name: redpanda-operator
-      image: docker.redpanda.com/redpandadata/redpanda-operator:v23.2.5


should we use 23.2.8 instead?

I wanted to merge this PR to run nightly and create PR from that run.

alejandroEsc · 2023-09-15T16:23:38Z

.github/bump_chart_versions.sh

+sed -i "s/^appVersion: .*$/appVersion: ${LATEST}/" "${CHARTFILE}"
+sed -i "s/${APPVERSION}/${LATEST}/" "${CHARTFILE}"


probably will need an extra blank line here,

Does shell need empty line at the end of the file?

alejandroEsc · 2023-09-15T16:23:58Z

charts/operator/templates/deployment.yaml

@@ -48,7 +48,7 @@ spec:
        - --metrics-bind-address=127.0.0.1:8080
        - --leader-elect
        - --configurator-tag={{ include "configurator.tag" . }}
-        - --configurator-base-image={{ .Values.configurator.repository }}
+        - --configurator-base-image={{ .Values.configurator.containerRegistry }}


why this change?

I was wondering that, too.

I highly recommend reading my commit message. I will put the whole reasoning into PR cover letter

operator: Change configurator container registry values yaml Currently nightly build is failing with the following error:

2023/09/15 01:26:17 Error querying repository tags: unable to open repository 'docker.redpanda.com/redpandadata/redpanda-operator
docker.redpanda.com/redpandadata/configurator': invalid reference: invalid repository

The values.yaml file for operator has 2 repository values which confuses `bump_chart_version.sh` script. The `docker-tag-list` program receives in the REPO variable the following content:

docker.redpanda.com/redpandadata/redpanda-operator
docker.redpanda.com/redpandadata/configurator

This change overcome this issue. P.S. New version of the Operator will not use configurator container. REF https://github.com/redpanda-data/helm-charts/actions/runs/6192749166/job/16813278284#step:6:7

joejulian · 2023-09-15T17:09:56Z

.github/bump_chart_versions.sh

+sed -i "s/^appVersion: .*$/appVersion: ${LATEST}/" "${CHARTFILE}"
+sed -i "s/${APPVERSION}/${LATEST}/" "${CHARTFILE}"


I was concerned that there could be a possibility that $APPVERSION might match some other string, like if we start adding release notes to the artifacthub metadata. It's also not quoting the dots so they'll be wildcards in this match.

I was thinking about maybe using yq (or gojq) to do this, roughly like:

IMAGES=$(yq -r '.annotations["artifacthub.io/images"]' charts/console/Chart.yaml) NEWIMAGES=$(yq -c --arg v $LATEST '(. | select(.name == "redpanda") | .image) |= "docker.redpanda.com/redpandadata/console:"+$v' <<< $IMAGES) yq -Y --argjson v $NEWIMAGES '.annotations["artifacthub.io/images"] = $v' charts/console/Chart.yaml > temp mv temp charts/console/Chart.yaml

joejulian

blocking on the breaking change

joejulian · 2023-09-15T17:11:33Z

charts/operator/values.yaml

@@ -24,7 +24,7 @@ image:

 configurator:
  # configurator.repository -- Repository that Redpanda configurator image is available
-  repository: docker.redpanda.com/redpandadata/configurator
+  containerRegistry: docker.redpanda.com/redpandadata/configurator


This is a breaking change. Is it needed?

I'm not that proficient in AWK to get the correct repository (container registry to be specific) from helm values yaml only if it comes from specific place. That place in values.yaml is not the same across different helm charts. The comment as far as see is different.

Currently nightly build is failing with the following error: ``` 2023/09/15 01:26:17 Error querying repository tags: unable to open repository 'docker.redpanda.com/redpandadata/redpanda-operator docker.redpanda.com/redpandadata/configurator': invalid reference: invalid repository ``` The values.yaml file for operator has 2 repository values which confuses `bump_chart_version.sh` script. The `docker-tag-list` program receives in the REPO variable the following content: ``` docker.redpanda.com/redpandadata/redpanda-operator docker.redpanda.com/redpandadata/configurator ``` This change overcome this issue. P.S. New version of the Operator will not use configurator container. REF https://github.com/redpanda-data/helm-charts/actions/runs/6192749166/job/16813278284#step:6:7

RafalKorepta · 2023-10-31T10:46:19Z

The #842 fixed operator issue

RafalKorepta requested review from joejulian and alejandroEsc as code owners September 15, 2023 11:52

RafalKorepta force-pushed the rk/fix-chart-version-bump branch from 04cac61 to 104caa1 Compare September 15, 2023 11:55

RafalKorepta mentioned this pull request Sep 15, 2023

bump version and bump annotation to proper version #724

Closed

alejandroEsc reviewed Sep 15, 2023

View reviewed changes

joejulian reviewed Sep 15, 2023

View reviewed changes

joejulian suggested changes Sep 15, 2023

View reviewed changes

RafalKorepta force-pushed the rk/fix-chart-version-bump branch from 104caa1 to 84dbacd Compare September 17, 2023 18:17

RafalKorepta requested a review from JakeSCahill as a code owner September 17, 2023 18:17

RafalKorepta added 5 commits September 18, 2023 16:40

console: Align artifacthub.io/images with the appVersion

c529c58

redpanda: Align artifacthub.io/images with the appVersion

44bdae5

operator: Align artifacthub.io/images with the appVersion

9a4cc6c

Add to chart version bump script annotations bump

204670f

RafalKorepta force-pushed the rk/fix-chart-version-bump branch from 84dbacd to 204670f Compare September 18, 2023 14:41

RafalKorepta mentioned this pull request Sep 19, 2023

update console appVersion from v2.3.2 to v2.3.3 #729

Merged

RafalKorepta closed this Oct 31, 2023

RafalKorepta deleted the rk/fix-chart-version-bump branch November 28, 2023 13:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Chart version bump fix #726

Chart version bump fix #726

RafalKorepta commented Sep 15, 2023 •

edited

Loading

alejandroEsc left a comment

alejandroEsc Sep 15, 2023

RafalKorepta Sep 15, 2023

alejandroEsc Sep 15, 2023

RafalKorepta Sep 15, 2023

alejandroEsc Sep 15, 2023

joejulian Sep 15, 2023

RafalKorepta Sep 15, 2023

joejulian Sep 15, 2023

joejulian left a comment

joejulian Sep 15, 2023

RafalKorepta Sep 15, 2023

RafalKorepta commented Oct 31, 2023

		sed -i "s/^appVersion: .*$/appVersion: ${LATEST}/" "${CHARTFILE}"
		sed -i "s/${APPVERSION}/${LATEST}/" "${CHARTFILE}"

Chart version bump fix #726

Chart version bump fix #726

Conversation

RafalKorepta commented Sep 15, 2023 • edited Loading

alejandroEsc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joejulian left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RafalKorepta commented Oct 31, 2023

RafalKorepta commented Sep 15, 2023 •

edited

Loading