Skip to content
This repository has been archived by the owner on Aug 17, 2023. It is now read-only.

Add support to kfctl to allow deployment on PPC64LE aka IBM Power architecture #290

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Changes from 2 commits
Commits
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: 12 additions & 4 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -126,14 +126,16 @@ build-kfctl: deepcopy generate fmt vet
#GOOS=windows GOARCH=amd64 ${GO} build -gcflags '-N -l' -ldflags "-X main.VERSION=$(TAG)" -o bin/windows/kfctl.exe cmd/kfctl/main.go
GOOS=darwin GOARCH=amd64 ${GO} build -gcflags '-N -l' -ldflags "-X main.VERSION=${TAG}" -o bin/darwin/kfctl cmd/kfctl/main.go
GOOS=linux GOARCH=amd64 ${GO} build -gcflags '-N -l' -ldflags "-X main.VERSION=$(TAG)" -o bin/linux/kfctl cmd/kfctl/main.go
cp bin/$(ARCH)/kfctl bin/kfctl
GOOS=linux GOARCH=ppc64le ${GO} build -gcflags '-N -l' -ldflags "-X main.VERSION=$(TAG)" -o bin/ppc64le/kfctl cmd/kfctl/main.go
#cp bin/$(ARCH)/kfctl bin/kfctl
Copy link
Member

Choose a reason for hiding this comment

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

Why are we commenting out this line? If it's no longer necessary, we should delete it.

Copy link
Member

Choose a reason for hiding this comment

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

@benswinney please address above comment from @Tomcli . That command is supposed to copy the platform specific kfctl executable to the default kfctl/bin directory. Any reason you are removing/commenting out that line?

Copy link
Author

@benswinney benswinney Mar 25, 2020

Choose a reason for hiding this comment

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

The architecture for kfctl on Darwin and amd64 linux is the same (x86_64). It was possible to use the same kfctl for both.

ppc64le is not the same architecture as Darwin and amd64, so you could not use the same kfctl file. We need to build separate images for ppc64le.

If the line is left in, then the final kfctl within bin/kfctl would be ppc64le and that would fail on x86_64 machines.

Hopefully this helps explain the need to remove that line.

Copy link
Author

Choose a reason for hiding this comment

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

I have updated the code again and pushed a new commit to remove the unneeded code :)


# Release tarballs suitable for upload to GitHub release pages
build-kfctl-tgz: build-kfctl
chmod a+rx ./bin/kfctl
chmod a+rx ./bin/$(ARCH)/kfctl
rm -f bin/*.tgz
cd bin/linux && tar -cvzf kfctl_$(TAG)_linux.tar.gz ./kfctl
cd bin/darwin && tar -cvzf kfctl_${TAG}_darwin.tar.gz ./kfctl
cd bin/ppc64le && tar -cvzf kfctl_${TAG}_ppc64le.tar.gz ./kfctl

build-and-push-operator: build-operator push-operator

Expand Down Expand Up @@ -168,13 +170,19 @@ push-to-github-release: build-kfctl-tgz
--repo kubeflow \
--tag $(TAG) \
--name "kfctl_$(TAG)_linux.tar.gz" \
--file bin/kfctl_$(TAG)_linux.tar.gz
--file bin/linux/kfctl_$(TAG)_linux.tar.gz
github-release upload \
--user kubeflow \
--repo kubeflow \
--tag $(TAG) \
--name "kfctl_$(TAG)_darwin.tar.gz" \
--file bin/kfctl_$(TAG)_darwin.tar.gz
--file bin/darwin/kfctl_$(TAG)_darwin.tar.gz
github-release upload \
--user kubeflow \
--repo kubeflow \
--tag $(TAG) \
--name "kfctl_$(TAG)_ppc64le.tar.gz" \
--file bin/ppc64le/kfctl_$(TAG)_ppc64le.tar.gz

build-kfctl-container:
DOCKER_BUILDKIT=1 docker build \
Expand Down