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.

Sign up for GitHub

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

Jump to bottom

Release script uses non-existent location for container tags #6220

Closed
absoludity opened this issue Apr 20, 2023 · 0 comments · Fixed by #6597
Closed

Release script uses non-existent location for container tags #6220

absoludity opened this issue Apr 20, 2023 · 0 comments · Fixed by #6597
Assignees
@antgamdia
Labels
component/ci Issue related to kubeapps ci system kind/bug An issue that reports a defect in an existing feature
Milestone
CI & E2E test imp...

Comments

@absoludity
Copy link
Contributor

Describe the bug

Our current release action tries to establish the latest tag for each upstream bitnami kubeapps image by querying the GH API for container repositories that no longer exist. In particular, these lines:

kubeapps/script/chart_sync_utils.sh

Lines 112 to 114 in 1e4634e

# Get the latest tag from the bitnami repository
local tag
tag=$(curl "${curl_opts[@]}" "https://api.github.com/repos/bitnami/${repoName}/tags" | jq -r '.[0].name')

attempt to reference, for example, a GH repo called bitnami/bitnami-docker-kubeapps-dashboard, which was deleted some months ago when the individual container repos were moved into bitnami/containers.

This change was announced in July last year, but the repos must have only been removed/deleted recently.

To Reproduce

Run the release action and find it fails with the less than obvious error:

Replacing dashboard...
jq: error (at <stdin>:4): Cannot index object with number
Error: Process completed with exit code 5.

Expected behavior

The upstream PR should be created without issue.
@absoludity absoludity added the kind/bug An issue that reports a defect in an existing feature label Apr 20, 2023
@absoludity absoludity mentioned this issue Apr 20, 2023
Merged
absoludity added a commit that referenced this issue Apr 20, 2023
@absoludity
Workaround for non-existent GH repo tags. (#6221)
77155ab 
<!--
Before you open the request please review the following guidelines and
tips to help it be more easily integrated:

 - Describe the scope of your change - i.e. what the change does.
 - Describe any known limitations with your change.
- Please run any tests or examples that can exercise your modified code.

 Thank you for contributing!
 -->

### Description of the change

See #6220 for a description.

### Benefits

We can release.

### Possible drawbacks

We'll need to manually update the auto-generated PR for the upstream
change with the correct images.

### Applicable issues


- ref #6220

### Additional information

Signed-off-by: Michael Nelson <minelson@vmware.com>
@absoludity absoludity mentioned this issue May 1, 2023
Merged
absoludity added a commit that referenced this issue May 1, 2023
@absoludity
Revert and fix so that latest tag is used in dev chart PRs. (#6237)
174b3d2 
<!--
Before you open the request please review the following guidelines and
tips to help it be more easily integrated:

 - Describe the scope of your change - i.e. what the change does.
 - Describe any known limitations with your change.
- Please run any tests or examples that can exercise your modified code.

 Thank you for contributing!
 -->

### Description of the change

After getting a couple of dev chart auto-created PRs like #6236, which
are doing nothing except setting our development images from `latest`
(which they should be in the dev chart) to a fixed version, I realised
that in #6221 I'd actually updated the incorrect part of the script.

This PR fixes that, by ensuring that we don't refer to `tag` in
`replaceImage_latestToProduction` since we don't have the production
values of the tag currently (until we update to use the dockerhub API as
per #6220) - this was the intended change of the previous PR - and
reverts the unintended change that updated
`replaceImage_productionToLatest` so that we again expect `latest` for
our own images in the dev chart.

### Benefits

<!-- What benefits will be realized by the code change? -->
We're no longer bothered by noop PRs such as #6236 .

### Possible drawbacks

<!-- Describe any known limitations with your change -->

### Applicable issues

<!-- Enter any applicable Issues here (You can reference an issue using
#) -->

- fixes #

### Additional information

<!-- If there's anything else that's important and relevant to your pull
request, mention that information here.-->

Signed-off-by: Michael Nelson <minelson@vmware.com>
@ppbaena ppbaena added this to the CI & E2E test improvements milestone May 8, 2023
@antgamdia antgamdia added the component/ci Issue related to kubeapps ci system label Aug 9, 2023
@antgamdia antgamdia self-assigned this Aug 9, 2023
@antgamdia antgamdia mentioned this issue Aug 9, 2023
Merged
antgamdia added a commit that referenced this issue Aug 10, 2023
@antgamdia
Fix script to fetch the latest published container in dockerhub (#6597)
11f2c09 
### Description of the change

This PR is to actually fix #6220 by fetching from DockerHub the more
recent (!=latest) published tag. I'm removing the workaround performed
in #6237.

### Benefits

The script will continue to work as initially expected.

### Possible drawbacks

N/A

### Applicable issues

- fixes #6220 

### Additional information

N/A

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@antgamdia antgamdia

Labels
component/ci Issue related to kubeapps ci system kind/bug An issue that reports a defect in an existing feature
Projects
Kubeapps
Archived in project
Milestone
CI & E2E test improvements
Development

Successfully merging a pull request may close this issue.

Fix script to fetch the latest published container in dockerhub antgamdia/kubeapps
4 participants
@absoludity @ppbaena @antgamdia and others