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

Dockerfile(arm64): Remove ETCD_UNSUPPORTED_ARCH #13847

Closed
wants to merge 1 commit into from

Conversation

jkroepke
Copy link

Remove ETCD_UNSUPPORTED_ARCH, since the platform is supported. See https://etcd.io/docs/v3.5/op-guide/supported-platform/

Remove ETCD_UNSUPPORTED_ARCH, since the platform is supported. See https://etcd.io/docs/v3.5/op-guide/supported-platform/
@jkroepke
Copy link
Author

CI failure

Step 1/14 : FROM ubuntu:21.10
toomanyrequests: You have reached your pull rate limit. You may increase the limit by authenticating and upgrading: https://www.docker.com/increase-rate-limit

@serathius
Copy link
Member

I don't know the context on why ETCD_UNSUPPORTED_ARCH was added here. cc @ptabor @spzala
Could you maybe find a PR that introduced the env variable here so we can make sure that original purpose is no longer relevant?

@jkroepke
Copy link
Author

@serathius git blame on github help me to identity it.

#12557

Look on the merge commit at that time, arm64 was not supported

if runtime.GOARCH == "amd64" || runtime.GOARCH == "ppc64le" || runtime.GOARCH == "s390x" {
return
}

Since arm64 is fully supported (#12929) now, I will consider this as leftover.

@serathius
Copy link
Member

I'm worried that arm is no longer covered by testing #13181, which means we should consider lowering it's tier.

cc @ptabor @spzala

@jkroepke
Copy link
Author

jkroepke commented Mar 29, 2022

Reading #13181

Will enable back once the maintenance is done.

Is was only temporary.? Tests should be re-enabled again?

Anyway. s390x is tier 3, but marked a supported. The discussion, if arm64 is tier 2 or tier 3 support does not relevant to this PR.

The Dockerfile for the s390x arch does not have the ENV variable, too.

@jkroepke
Copy link
Author

jkroepke commented Apr 4, 2022

@serathius What did you think? i guess the discussion about the support tier is not affected by the change. See last post.

@serathius
Copy link
Member

Reading #13181

Will enable back once the maintenance is done.

Is was only temporary.? Tests should be re-enabled again?

Those tests run on custom workers provided by one of the maintainers that is no longer active. To re-enable those tests we would need to find an alternative way to get arm64 workers.

Anyway. s390x is tier 3, but marked a supported. The discussion, if arm64 is tier 2 or tier 3 support does not relevant to this PR.

The Dockerfile for the s390x arch does not have the ENV variable, too.

I don't know if the tiers listed on https://etcd.io/docs/v3.5/op-guide/supported-platform/ are up to date. cc @spzala

@jkroepke
Copy link
Author

jkroepke commented Jul 7, 2022

@ptabor @spzala are you able to provide input here?

@spzala
Copy link
Member

spzala commented Aug 24, 2022

I don't know the context on why ETCD_UNSUPPORTED_ARCH was added here. cc @ptabor @spzala Could you maybe find a PR that introduced the env variable here so we can make sure that original purpose is no longer relevant?

@serathius sorry, missed it earlier, here is a related PR - #12557

@spzala
Copy link
Member

spzala commented Aug 24, 2022

Reading #13181

Will enable back once the maintenance is done.

Is was only temporary.? Tests should be re-enabled again?

Those tests run on custom workers provided by one of the maintainers that is no longer active. To re-enable those tests we would need to find an alternative way to get arm64 workers.

Anyway. s390x is tier 3, but marked a supported. The discussion, if arm64 is tier 2 or tier 3 support does not relevant to this PR.
The Dockerfile for the s390x arch does not have the ENV variable, too.

I don't know if the tiers listed on https://etcd.io/docs/v3.5/op-guide/supported-platform/ are up to date. cc @spzala

Yes, agree. Those tests couldn't be re-enabled for a security reason and cannot be enabled any more.

@spzala
Copy link
Member

spzala commented Aug 24, 2022

I'm worried that arm is no longer covered by testing #13181, which means we should consider lowering it's tier.

cc @ptabor @spzala

@serathius yes. I saw you have made doc changes which makes sense, thanks!

Copy link
Member

@spzala spzala left a comment

Choose a reason for hiding this comment

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

@jkroepke thanks for the PR and discussion. Considering the removal of related tests and unstable support, I think we should not make any changes as they were added to fix a real issue in the past. cc @serathius

@jkroepke
Copy link
Author

Is there a reason, why the sx390 and ppc images are not flagged as unsupported?

@serathius
Copy link
Member

I think we just forgot to make it consistent across images. #12543

@jkroepke
Copy link
Author

Okay, I got it. In conclusion, I will close this PR now.

@jkroepke jkroepke closed this Aug 24, 2022
@jkroepke jkroepke deleted the patch-1 branch August 24, 2022 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants