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

Add 2.x appVersion as optional version for helm charts #257

Conversation

peterzhuamazon
Copy link
Member

@peterzhuamazon peterzhuamazon commented May 3, 2022

Signed-off-by: Peter Zhu [email protected]

Description

Add 2.x appVersion as optional version for helm charts

Issues Resolved

opensearch-project/opensearch-build#1624

Check List

  • Commits are signed per the DCO using --signoff

For any changes to files within Helm chart directories:

  • Helm chart version bumped
  • Helm chart CHANGELOG.md updated to reflect change

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@peterzhuamazon peterzhuamazon requested a review from a team as a code owner May 3, 2022 18:55
@peterzhuamazon peterzhuamazon self-assigned this May 3, 2022
Signed-off-by: Peter Zhu <[email protected]>
@peterzhuamazon
Copy link
Member Author

@DandyDeveloper @TheAlgo @prudhvigodithi we need to think about how to support multiple versions.
As of now I dont think adding version branch is better than just allowing user to switch version themselves.
But I welcome ideas on this one.

The thing is, our release action will keep pushing new chart versions from main.
This means if we change appVersion to 2.0.0-rc1 now two days later 1.3.2 release will change it back to 1.x.
Which is not ideal at all.

Thanks.

Signed-off-by: Peter Zhu <[email protected]>
@prudhvigodithi
Copy link
Member

prudhvigodithi commented May 3, 2022

Hey @peterzhuamazon, thats true.

  • Main branch should always point to latest release (Consider main as a development branch).
  • We already have tags to use a specific version.

So lets say if we have 2.0.0-rc1 release, main should have 2.0.0-rc1, we need to increment the version to 2.x and cut a tag with this, user should refer to tag rather than main branch. Same goes with 1.3.2, main should have latest release 1.3.2 and cut a tag with 1.x, so that users can use this tag who still uses 1.3.x version.

we can start to follow this with existing PR, we can have appVersion: "2.0.0-rc1".

@peterzhuamazon
Copy link
Member Author

Per @bbarani we are not switching the current running version to rc1 and will keep as 1.3.1 until ga.
This PR is still useful to allow users to know there is an option.
Thanks.

@prudhvigodithi
Copy link
Member

prudhvigodithi commented May 3, 2022

Per @bbarani we are not switching the current running version to rc1 and will keep as 1.3.1 until ga.
This PR is still useful to allow users to know there is an option.
Thanks.

Then we might need to create a tag manually as opensearch-2.0.0-rc1 (that has appVersion: "2.0.0-rc1"), so that users can directly use this tag for 2.0.0-rc1, thoughts @bbarani and @peterzhuamazon ?

@TheAlgo
Copy link
Member

TheAlgo commented May 4, 2022

My 2 cents:

  1. Adding a documented line item will not make much sense to me , this is something as default version. This can ideally point to any OpenSearch version. We can have proper documentation regarding how to upgrade versions via helm charts and maybe maintain a compatibility matrix of the helm chart version and the OpenSearch version if any. Ideally main should point to latest release.

  2. Also we should invest some effort in better release notes [Enhancement][opensearch] Release notes included in the GitHub Release Page #173

  3. On the release I am still of the opinion to not release so many chart versions and rather have release branches for charts which we can release periodically and non periodically in case of urgencies. Check comments in [CI] Need automate version bump PR to be opened once OpenSearch/Dashboards release new versions #205

@peterzhuamazon
Copy link
Member Author

Hi,

Thanks @TheAlgo @prudhvigodithi on your suggestions here.
We still have some time before 2.0.0 GA comes.
This PR can stay here for a bit and I will probably put back to draft.

@smlx @mprimeaux what is your take on this one?

Thanks.

@smlx
Copy link
Contributor

smlx commented May 5, 2022

What is the support policy for Opensearch 1.x upstream? Will they be doing further 1.x releases? Will it be getting security patches for some period of time? If so you might need a 1.x branch.

@peterzhuamazon
Copy link
Member Author

What is the support policy for Opensearch 1.x upstream? Will they be doing further 1.x releases? Will it be getting security patches for some period of time? If so you might need a 1.x branch.

I believe we will still support 1.x branch with patches, due to 2.x have a lot of breaking changes.

My two cents:

  • The branch main will have version 2.x.x chartversion+appversion, and allow the workflow to auto create releases just like before.
  • Create 1.x branch, will have version 1.x.x chartverion+appversion. The problem with this is the workflow will only publish anything on main to github-pages, unless we can support two pages we need to add instructions to use charts with tgz like in ODFE. Add a workflow to create tags for 1.x but not releases.

Thoughts?
@DandyDeveloper @TheAlgo @prudhvigodithi @smlx @mprimeaux ?

Thanks.

@peterzhuamazon peterzhuamazon marked this pull request as draft May 5, 2022 16:25
@smlx
Copy link
Contributor

smlx commented May 5, 2022

I believe we will still support 1.x branch with patches, due to 2.x have a lot of breaking changes.

Ok, thanks for confirming.

My two cents:

* The branch `main` will have version `2.x.x` chartversion+appversion, and allow the workflow to auto create releases just like before.

* Create `1.x` branch, will have version `1.x.x` chartverion+appversion.

Agree that this seems like a reasonable solution.

The problem with this is the workflow will only publish anything on main to github-pages, unless we can support two pages we need to add instructions to use charts with tgz like in ODFE. Add a workflow to create tags for 1.x but not releases.

I think you can update the workflow to also publish releases from the 1.x branch to the same page by adding 1.x to the list here. This might need some testing though.

@peterzhuamazon
Copy link
Member Author

I believe we will still support 1.x branch with patches, due to 2.x have a lot of breaking changes.

Ok, thanks for confirming.

My two cents:

* The branch `main` will have version `2.x.x` chartversion+appversion, and allow the workflow to auto create releases just like before.

* Create `1.x` branch, will have version `1.x.x` chartverion+appversion.

Agree that this seems like a reasonable solution.

The problem with this is the workflow will only publish anything on main to github-pages, unless we can support two pages we need to add instructions to use charts with tgz like in ODFE. Add a workflow to create tags for 1.x but not releases.

I think you can update the workflow to also publish releases from the 1.x branch to the same page by adding 1.x to the list here. This might need some testing though.

I am not completely sure on that @smlx.
The reason I am saying that is because internally the action will push a branch to gh-pages. If we push both main and 1.x to gh-pages one will override the other.

Also, I dont think github allows you to host two urls for 1 repo.
If we only have one url how can we point to 2 different versions?

And yes we do need some testing on this.
@prudhvigodithi will post another issue soon which includes some findings and proposals based on my initial thoughts, on how to do this the best way soon.

Thanks.

@smlx
Copy link
Contributor

smlx commented May 6, 2022

I am not completely sure on that @smlx. The reason I am saying that is because internally the action will push a branch to gh-pages. If we push both main and 1.x to gh-pages one will override the other.

The chart-releaser-action updates index.yaml on the same gh-pages branch, rather than overwriting it.

The ingress-nginx repo does this, take a look at their workflow here, and commits on the gh-pages branch which updated the index.yaml for a chart release. Here are two example updates: legacy, and main.

Maybe you could ask them for advice on this?

Also, I dont think github allows you to host two urls for 1 repo. If we only have one url how can we point to 2 different versions?

All the versions are added to the index.yaml.

And yes we do need some testing on this. @prudhvigodithi will post another issue soon which includes some findings and proposals based on my initial thoughts, on how to do this the best way soon.

Sounds good 🙂

@prudhvigodithi
Copy link
Member

prudhvigodithi commented May 6, 2022

Hey here is the issue #259 I have created with some solutions detailed, please check @TheAlgo @DandyDeveloper @peterzhuamazon @smlx @bbarani

@TheAlgo
Copy link
Member

TheAlgo commented May 6, 2022

My two cents:
The branch main will have version 2.x.x chartversion+appversion, and allow the workflow to auto create releases just like before.
Create 1.x branch, will have version 1.x.x chartverion+appversion. The problem with this is the workflow will only publish anything on main to github-pages, unless we can support two pages we need to add instructions to use charts with tgz like in ODFE. Add a workflow to create tags for 1.x but not releases.

@peterzhuamazon I partially agree with you. We can have a branch for 1.x, but lets have individual releases for 1.x and 2.x. Also seeing the other comments I see this is feasible and we can maintain 2 branch release same as kubernetes has done for ingress-nginx. Going forward I believe this is a good practice to have branches representing major versions (1.x, 2.x, 3.x so on and so forth).

@peterzhuamazon
Copy link
Member Author

Thanks everyone, since the issue is up will close this and move the discussion to there.

@peterzhuamazon peterzhuamazon deleted the opensearch-2.0.0-release branch May 7, 2022 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants