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

Support arm64 when running integration tests #1634

Closed
codefromthecrypt opened this issue Apr 11, 2024 · 10 comments · Fixed by #1635
Closed

Support arm64 when running integration tests #1634

codefromthecrypt opened this issue Apr 11, 2024 · 10 comments · Fixed by #1635
Milestone

Comments

@codefromthecrypt
Copy link
Contributor

codefromthecrypt commented Apr 11, 2024

Is your feature request related to a problem? Please describe.

Fabric8IstioIT uses an x64 binary checked into source control. This can create problems when trying to use docker on Apple Silicon, particularly nested VMs:

spring-cloud-kubernetes-test-support/src/main/resources/istio-cli/istio-1.16.0/bin/istioctl

Describe the solution you'd like

It is likely a better solution to copy this from the image pulled instead of checking 80MB binaries into source control

Describe alternatives you've considered

An alternative would be to also check in an arm64 binary

@wind57
Copy link
Contributor

wind57 commented Apr 11, 2024

some time ago, we had such code in place in that integration test:

// for Mac M1 with aarch64
		if (System.getProperty("os.arch").equals("aarch64")) {
			processExecResult(K3S.execInContainer("sh", "-c", CONTAINER_ISTIO_BIN_PATH + "istioctl"
					+ " --kubeconfig=/etc/rancher/k3s/k3s.yaml install --set hub=docker.io/querycapistio --set profile=minimal -y"));
		}

The reason for that was simple: before istio 1.16 there was no support for arm64, and we had to somehow run that locally. I'm on Mac M1 too, so is Ryan, iirc.

Since we upgraded to 1.16, there is no need for that anymore, so we dropped that check.


I don't understand your point about:

copy this from the image pulled

what image? k3s? If so, it does not have istio or istioctl.

@wind57
Copy link
Contributor

wind57 commented Apr 11, 2024

I'm confused even more now :) its most probably the fact that its over 23:00 where I am, so sorry upfront.

How can I delete those, if I need istioctl, and I don't have it in the container?

My point above was that we don't need that check anymore because since 1.16 it works on arm64, but we still need istioctl, of course

@codefromthecrypt
Copy link
Contributor Author

The file checked into version control, spring-cloud-kubernetes-test-support/src/main/resources/istio-cli/istio-1.16.0/bin/istioctl is an x64 binary (you can verify by doing file on that).

If it works on an arm64 machine, there's an assumption of x64 emulation available (and also in the docker runtime).

Maybe in the morning, re-read this part if it doesn't make sense now ;) I think that it isn't typical or correct to depend on being to emulate a specific architecture, even if it was ok to check in an 80MB binary.

@wind57
Copy link
Contributor

wind57 commented Apr 11, 2024

ah! I see your point, so you basically want to run this test locally, on your Mac M1, but that is a x64 binary... makes sense now. I'll need to think about it for a while, because I don't think we want another binary in the source control specific to arm64

@codefromthecrypt
Copy link
Contributor Author

I don't think we want another binary in the source control specific to arm64

Right, so what I meant was to get it from an image, like istiod's docker image. The image you pull should already be the correct arch, so the binary in there should be the right one.

@codefromthecrypt
Copy link
Contributor Author

so istio/istioctl:1.21.1 has it

$ car --platform=linux/amd64 -tvf istio/istioctl:1.21.1 |grep istioctl
-rwxr-xr-x	81383576	Apr  5 06:39:43	usr/local/bin/istioctl

@wind57
Copy link
Contributor

wind57 commented Apr 11, 2024

yup, I saw that one too. working on it, let's see...

@codefromthecrypt
Copy link
Contributor Author

will be something like this:

		// Copy istioctl to the k3s container
		DockerClient client = K3S.getDockerClient();
		String ISTIO_ISTIOCTL = "istio/istioctl";
		Commons.pullImage(ISTIO_ISTIOCTL, Commons.ISTIO_VERSION, K3S);
		try (CreateContainerCmd createCmd = client.createContainerCmd(ISTIO_ISTIOCTL + ":" + Commons.ISTIO_VERSION)) {
			CreateContainerResponse container = createCmd.
				withEntrypoint("/bin/sh", "-c").
				withCmd("while true; do sleep 1000; done").
				exec();
			try (StartContainerCmd startCmd = client.startContainerCmd(container.getId())) {
				startCmd.exec();
				try (CopyArchiveFromContainerCmd copyCmd = client.copyArchiveFromContainerCmd(container.getId(), "/usr/local/bin/istioctl");
					InputStream response = copyCmd.exec()) {
					K3S.copyFileToContainer(
						Transferable.of(response.readAllBytes()),
						"/tmp/istioctl"
					);
				}
			}
			finally { // Stop and remove the container
				K3S.getDockerClient().stopContainerCmd(container.getId()).exec();
				K3S.getDockerClient().removeContainerCmd(container.getId()).exec();
			}
		}

@ryanjbaxter ryanjbaxter linked a pull request Apr 12, 2024 that will close this issue
@ryanjbaxter ryanjbaxter added this to the 3.1.2 milestone Apr 12, 2024
@codefromthecrypt
Copy link
Contributor Author

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants