opensearch-project / opensearch-k8s-operator

OpenSearch Kubernetes Operator
Apache License 2.0
366 stars 192 forks source link

Upgrade Project go to `1.22` and Remove `allow_auto_create` option #767

Closed prudhvigodithi closed 3 months ago

prudhvigodithi commented 3 months ago

Description

The allow_auto_create does not exist in the OpenSearch Core code for _component_templates.

Issues Resolved

Part of https://github.com/opensearch-project/OpenSearch/issues/12937, https://github.com/opensearch-project/opensearch-k8s-operator/issues/768 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.

prudhvigodithi commented 3 months ago

Updated the project go version to 1.22 to ensure envtest-setup setup-envtest@latest works (hard blocker). Please check this was recently moved to go 1.22 https://github.com/kubernetes-sigs/controller-runtime/commit/4c2442e4d743d172a2e0126a841ecd274fffc228#diff-d9a956120ac84289d2650137f64bd8f0893331de05e44cc41899dd984c9e0d48

swoehrl-mw commented 3 months ago

@prudhvigodithi Didn't have time to look at the whole PR, but this one jumped out: Removing a field from the CRD is a breaking change and would require a major release. It would be easier to keep the field in the CRD for compatibility and just ignore it when communicating with the Opensearch API.

salyh commented 3 months ago

@prudhvigodithi Didn't have time to look at the whole PR, but this one jumped out: Removing a field from the CRD is a breaking change and would require a major release. It would be easier to keep the field in the CRD for compatibility and just ignore it when communicating with the Opensearch API.

I agree with @swoehrl-mw - Otherwise it looks good although we should upgrade to 1.22.1 like @sunsrin suggested

prudhvigodithi commented 3 months ago

@prudhvigodithi do you want to use 1.22.1 - which is the latest stable release with few more security fixes - https://tip.golang.org/doc/devel/release#go1.22.minor or do you want to stick to major versions only ? thank you for looking into this - fell off my radar

Hey @sunsrin let me check with 1.22.1.

prudhvigodithi commented 3 months ago

Sure @swoehrl-mw @salyh let me check to ignore the field still keeping the CRD, I will try to add a logging when used allow_auto_create saying its not supported in OpenSearch, this way user will know that till is not applied for _component_templates. @bbarani

prudhvigodithi commented 3 months ago

I have updated the code to retain the AllowAutoCreate, but when applied shows the following warning to the user, the allow_auto_create will not be adding to the _component_template

{"level":"debug","ts":"2024-03-28T13:03:49.105-0700","logger":"events","msg":"OpenSearch Component Index template does not support allow_auto_create","type":"Warning","object":{"kind":"OpensearchComponentTemplate","namespace":"default","name":"default-component-template","uid":"1286a7b0-39d2-400e-b165-ce7f6916a466","apiVersion":"opensearch.opster.io/v1","resourceVersion":"22502365"},"reason":"OpensearchAPIUpdated"}
prudhvigodithi commented 3 months ago

Hey @sunsrin I have updated to 1.22.1, looks good.

prudhvigodithi commented 3 months ago

Thanks @sunsrin, @swoehrl-mw @salyh are we good to move forward ?