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

Upgrade kustomize version (v3.8.7 to v4.5.2) to support ppc64le/s390x #5674

Closed
wants to merge 3 commits into from
Closed

Upgrade kustomize version (v3.8.7 to v4.5.2) to support ppc64le/s390x #5674

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b722b4c
Upgrade kustomize version (v3.8.7 to v4.5.2) to support ppc64le and s…
gquillar Apr 21, 2022
e0eb2d5
Upgrade kustomize version (v3.8.7 to v4.5.2) to support ppc64le and s…
gquillar Apr 21, 2022
49982fe
Merge branch 'fix_kustomize_multi_arch' of https://github.com/gquilla…
gquillar Apr 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions changelog/fragments/bump-kustomize.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# entries is a list of entries to include in
# release notes and/or the migration guide
entries:
- description: >
(ansible/v1, helm/v1) Bumping Kuztomize version in project scaffolding to 4.5.2.

# kind is one of:
# - addition
# - change
# - deprecation
# - removal
# - bugfix
kind: "change"

# Is this a breaking change?
breaking: false
Copy link
Contributor

Choose a reason for hiding this comment

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

It is a breaking change

2 changes: 1 addition & 1 deletion internal/plugins/ansible/v1/scaffolds/init.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import (

const (
// kustomizeVersion is the sigs.k8s.io/kustomize version to be used in the project
kustomizeVersion = "v3.8.7"
kustomizeVersion = "v4.5.2"
Copy link
Contributor

Choose a reason for hiding this comment

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

who should know what the kustomize version used/support is the plugin responsible to generate/scaffolding the kustomize files since its manifests need to be compatible with. Also, it does not seem to fit under the domain of resposanbility of ansible/helm/go plugins.

Who is responsible for the kustomize is the kustomize plugin.
See that we are proposing to fix it in the PR, see:

So that, we could do dp the same fix here.

Copy link
Author

Choose a reason for hiding this comment

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

Since that moving to kustomize v4 requires to use the new plugin kustomize/v2-alpha which is still in development (PR-2583) and will be unstable (alpha version), I think that it is better to build locally the kustomize 3.8.7 for the not supported architectures as suggested by @natasha41575 in PR-4612.

So @camilamacedo86 for ppc64le and s390x, I propose to not move to v4.5.4 but rather use a locally built kustomize v3.8.7. If you agree, I'll do the following change in internal/plugins/ansible/v1/scaffolds/internal/templates/makefile.go and internal/plugins/helm/v1/scaffolds/internal/templates/makefile.go to generate the Makefile for ansible and helm operator samples.

.PHONY: kustomize
KUSTOMIZE = $(shell pwd)/bin/kustomize
kustomize: ## Download kustomize locally if necessary.
ifeq (,$(wildcard $(KUSTOMIZE)))
ifeq (,$(shell which kustomize 2>/dev/null))
        @{ \
        set -e ;\
        mkdir -p $(dir $(KUSTOMIZE)) ;\
-       curl -sSLo - https://github.com/kubernetes-sigs/kustomize/releases/download/kustomize/{{ .KustomizeVersion }}/kustomize_{{ .KustomizeVersion }}_$(OS)_$(ARCH).tar.gz | \
-       tar xzf - -C bin/ ;\
+       $(eval arch = $(shell uname -m)) \
+       if [ "${arch}" = "ppc64le" -o "${arch}" = "s390x" ]; then \
+               $(eval tmpDir = $(shell mktemp -d)) \
+               curl -sSLo - https://api.github.com/repos/kubernetes-sigs/kustomize/tarball/kustomize/v3.8.7 | \
+               tar xzf - -C ${tmpDir} --wildcards kubernetes-sigs-kustomize-\*/kustomize ;\
+               cd ${tmpDir}/kubernetes-sigs-kustomize-*/kustomize && GOBIN=$(dir $(KUSTOMIZE)) go install . && cd - ;\
+               rm -rf ${tmpDir} ;\
+       else \
+               curl -sSLo - https://github.com/kubernetes-sigs/kustomize/releases/download/kustomize/v3.8.7/kustomize_v3.8.7_$(OS)_$(ARCH).tar.gz | \
+               tar xzf - -C bin/ ;\
+       fi \
        }
else
KUSTOMIZE = $(shell which kustomize)
endif
endif

Copy link
Contributor

@camilamacedo86 camilamacedo86 May 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

You can change your scaffold as please you.
However, to support these archs we need to more than only be able to have kustomize to them, such as:

  • All images scaffolds by default must support them
  • For golang kubebuilder-tools which ships what is required to use env test also must support

(ihmo) we need to address all changes in Kubebuilder.
When kubebuilder be able to support then we can update SDK.
We might be able to make ansible/helm support first but it might reduce the maintainability


imageName = "controller:latest"
)
Expand Down
2 changes: 1 addition & 1 deletion internal/plugins/helm/v1/scaffolds/init.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import (

const (
// kustomizeVersion is the sigs.k8s.io/kustomize version to be used in the project
kustomizeVersion = "v3.8.7"
kustomizeVersion = "v4.5.2"
Copy link
Contributor

@camilamacedo86 camilamacedo86 May 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

(imo) We need to change add to Kubebuilder : kubernetes-sigs/kubebuilder#2583.
After that, we will need to check if we can or not upgrade the current ansible/helm plugins with or we will need alpha versions of them.
See that the kustomize version should not be in ansbile/helm/golang plugin, https://github.com/kubernetes-sigs/kubebuilder/pull/2583/files#r839541438 (it is part of the kustomize plugin domain of responsability)


imageName = "controller:latest"
)
Expand Down
2 changes: 1 addition & 1 deletion testdata/ansible/memcached-operator/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ ifeq (,$(shell which kustomize 2>/dev/null))
@{ \
set -e ;\
mkdir -p $(dir $(KUSTOMIZE)) ;\
curl -sSLo - https://github.com/kubernetes-sigs/kustomize/releases/download/kustomize/v3.8.7/kustomize_v3.8.7_$(OS)_$(ARCH).tar.gz | \
curl -sSLo - https://github.com/kubernetes-sigs/kustomize/releases/download/kustomize/v4.5.2/kustomize_v4.5.2_$(OS)_$(ARCH).tar.gz | \
tar xzf - -C bin/ ;\
}
else
Expand Down
2 changes: 1 addition & 1 deletion testdata/helm/memcached-operator/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ ifeq (,$(shell which kustomize 2>/dev/null))
@{ \
set -e ;\
mkdir -p $(dir $(KUSTOMIZE)) ;\
curl -sSLo - https://github.com/kubernetes-sigs/kustomize/releases/download/kustomize/v3.8.7/kustomize_v3.8.7_$(OS)_$(ARCH).tar.gz | \
curl -sSLo - https://github.com/kubernetes-sigs/kustomize/releases/download/kustomize/v4.5.2/kustomize_v4.5.2_$(OS)_$(ARCH).tar.gz | \
tar xzf - -C bin/ ;\
}
else
Expand Down