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

Update docker-compose.yml. #2612

Merged
merged 2 commits into from
Sep 14, 2022
Merged

Update docker-compose.yml. #2612

merged 2 commits into from
Sep 14, 2022

Conversation

Yury-Fridlyand
Copy link
Contributor

Signed-off-by: Yury-Fridlyand yuryf@bitquilltech.com

Description

The proposed changes contain updated docker-compose.yml which doesn't use obsolete OpenSearch settings (see opensearch-project/OpenSearch#2463).

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.

Signed-off-by: Yury-Fridlyand <yuryf@bitquilltech.com>
@prudhvigodithi
Copy link
Member

Hey @Yury-Fridlyand thanks for the PR, this breaks next 1.3.x series, should be have a prefix folders? or a new docker-compose file that distinguishes 1.3.x and above 2.x? @peterzhu1992 @gaiksaya @bbarani @dblock

@peterzhuamazon
Copy link
Member

peterzhuamazon commented Sep 13, 2022

Hey @Yury-Fridlyand thanks for the PR, this breaks next 1.3.x series, should be have a prefix folders? or a new docker-compose file that distinguishes 1.3.x and above 2.x? @peterzhu1992 @gaiksaya @bbarani @dblock

We can definitely separated to different folder.
It wont really break us as we dont use this file actively, just reference.
The one on the website and documentation is more of an issue, they probably need some backport to 2.0, 2.1, 2.2 branches.

I propose rename this as docker-compose-2.x.yml, and restore the old one as docker-compose-1.x.yml.

The reason being the original change by @tlfeng was for 2.0, so assuming this change is already in every 2.x.

Thanks.

Signed-off-by: Yury-Fridlyand <yuryf@bitquilltech.com>
@Yury-Fridlyand
Copy link
Contributor Author

Done. I can't find any references to this file. Please, check that no script nor readme require an update.

Thanks!

@prudhvigodithi
Copy link
Member

LGTM! @dblock @peterzhuamazon @gaiksaya, can you review as well?
Thank you

@peterzhuamazon
Copy link
Member

Done. I can't find any references to this file. Please, check that no script nor readme require an update.

Thanks!

Hi @Yury-Fridlyand,

My wording might be a bit off.
When I say reference I meant to say that the opensearch.org website/documentation ones are copied from here.
So this is where the file first being created.

Your change is well and done in this repo now.
Thanks very much.

@peterzhuamazon
Copy link
Member

LGTM! @dblock @peterzhuamazon @gaiksaya, can you review as well? Thank you

I will keep this open a bit until documentation team confirm this is confirmed valid, since I have not tested this yet.
If @tlfeng can give his opinion it would be great.

Thanks.

@tlfeng
Copy link
Contributor

tlfeng commented Sep 14, 2022

@peterzhuamazon I confirm that the cluster setting name cluster.initial_master_nodes is deprecated in version 2.0, and the alternative name is cluster.initial_cluster_manager_nodes, which is introduced in the commit opensearch-project/OpenSearch@19eadb4.
The old setting name cluster.initial_master_nodes is planned to be removed in version 3.0, and the new setting name cluster.initial_cluster_manager_nodes is available in version 2.0 and above.

Some of our plugins have been using the new name in their CI for testing (https://github.com/opensearch-project/anomaly-detection-dashboards-plugin/blob/2.1.0.0/.github/configurations/docker-compose.yml), so I think the setting name change in this PR will work.

@peterzhuamazon
Copy link
Member

@peterzhuamazon I confirm that the cluster setting name cluster.initial_master_nodes is deprecated in version 2.0, and the alternative name is cluster.initial_cluster_manager_nodes, which is introduced in the commit opensearch-project/OpenSearch@19eadb4. The old setting name cluster.initial_master_nodes is planned to be removed in version 3.0, and the new setting name cluster.initial_cluster_manager_nodes is available in version 2.0 and above.

Some of our plugins have been using the new name in their CI for testing (https://github.com/opensearch-project/anomaly-detection-dashboards-plugin/blob/2.1.0.0/.github/configurations/docker-compose.yml), so I think the setting name change in this PR will work.

Thanks @tlfeng.

@peterzhuamazon
Copy link
Member

Thanks @Yury-Fridlyand we will leave this PR unmerged until project website and documentation maintainers merged on their side.

Approved now.

@peterzhuamazon peterzhuamazon merged commit 0ada2cd into opensearch-project:main Sep 14, 2022
@Yury-Fridlyand Yury-Fridlyand deleted the update-docker-compose branch September 14, 2022 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants