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

Fix dev environment after bitnami/kubeapps chart update. #2828

Merged
merged 4 commits into from
May 18, 2021

Conversation

absoludity
Copy link
Contributor

@absoludity absoludity commented May 18, 2021

Description of the change

The recent update of the bitnami/kubeapps chart, which we synced, had a backwards incompatible change with the dev environment.

Specifically, the extraTls option now needs to be used to specify an existing TLS secret for ingress.

While there, I updated the target to ensure that it can be run multiple times to continue applying local changes (ie. ensured we create a secret for the postgresql secret also).

UPDATE: I ended up needing a local env with pinniped to investigate #2827 , so fixed the similar issue in the pinniped make targets. I also cleaned them up a bit (no more deleting/creating secrets, instead using the created secret), removed one yaml file which was near identical to the non-pinniped one (so just re-used that and set the one option to enable pinniped). Also fixed the upstream chart issue here for #2826, I'll create a separate PR upstream for that.

Benefits

Easy dev with OIDC (on linux) works again.

Possible drawbacks

None.

@absoludity absoludity added this to In progress in Kubeapps via automation May 18, 2021
@absoludity absoludity moved this from In progress to Waiting For Review in Kubeapps May 18, 2021
@@ -25,4 +25,4 @@ maintainers:
name: kubeapps
sources:
- https://github.com/kubeapps/kubeapps
version: 7.0.0
version: 7.0.1-dev1
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@antgamdia I bumped the chart here, so it's a pre-release of 7.0.1 (ie, < 7.0.1 according to semver), but the important thing is that this shouldn't trigger a PR to the upstream chart either right? (as in, the comparison for creating the upstream chart would need to compare with the current release + 0.0.1 (ie. check if 7.0.1-dev1 > 7.0.1, which it's not... rather than checking if 7.0.1-dev1 > 7.0.0, which it is). Does that make sense? I'll just revert the chart bump here if you don't have time to deal with that for now.

Copy link
Contributor

Choose a reason for hiding this comment

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

this shouldn't trigger a PR to the upstream chart either right?
No, our local changes will NOT trigger any update in the bitnami/charts repo, unless we are releasing (i.e., creating a tag)

(as in, the comparison for creating the upstream chart would need to compare with the current release + 0.0.1 (ie. check if 7.0.1-dev1 > 7.0.1, which it's not... rather than checking if 7.0.1-dev1 > 7.0.0, which it is).

What we are doing on every push a master is to check:

# before bitnami accepts the PR, the remote version will be 7.0.0
semverCompare=$(semver compare "7.0.1-dev1" "7.0.0") # 1
  # Do nothing, our chart is more updated than the remote one, that is OK
...

# after bitnami accepts the PR, the remote version will be 7.0.1
semverCompare=$(semver compare "7.0.1-dev1" "7.0.1") # -1
# The current version is less than the chart external version, then retrieve the changes and send an internal PR with them
# But, given that some changes are already there, we will have a 7.0.1 version with almost no changes.
# In fact, if it is the same set of changes, we only get a chart version update.

Does that make sense?

Yes, and no. I mean: it is a bit odd sending duplicated PRs in both repos. If we happen to add any other change, we will have a, let's say, version 7.0.1 which does not match the upstream one. In that case, we would have to manually modify the PR and set it to 7.0.2-dev.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perfect. For some reason I thought we were sending a PR the other way if we intentionally made the chart version greater... but what you've described is much saner: we tag a release to trigger a PR to bitnami/charts, while we pull any new chart changes down. Great, thanks.

@absoludity absoludity merged commit 38942ea into vmware-tanzu:master May 18, 2021
Kubeapps automation moved this from Waiting For Review to Done May 18, 2021
@absoludity absoludity deleted the fix-dev branch May 18, 2021 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Kubeapps
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants