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

[JUJU-3600] Remove 32-bit architecture support #15534

Merged
merged 8 commits into from
Apr 28, 2023

Conversation

SimonRichardson
Copy link
Member

@SimonRichardson SimonRichardson commented Apr 25, 2023

This is the first step in fixing the problematic arch supported list from LXD. Whilst investigating that bug, it seems that we still support 32-bit word sizes. Except that, we no longer want to support 32-bit word sizes with dqlite, as the number of jujud permutations explodes and so it's best to completely remove the support. We officially dropped support for 32-bit architectures over 3 years ago, although we didn't actively remove the code to drop support.

This brings in what the juju/utils arch support had, with the addition of a UnsupportedArches field, which will ensure that we can at least say what we use to support, but no longer do. This doesn't bring in the changes from juju/utils PR as technically we don't need a new version of juju/utils sans arch package to do this.

Checklist

  • Code style: imports ordered, good names, simple structure, etc
  • Comments saying why design decisions were made
  • Go unit tests, with comments saying what you're testing

QA steps

$ juju bootstrap lxd test --build-agent

Bug reference

https://bugs.launchpad.net/juju/+bug/2013869

The following removes 32-bit architecture support. Although we've
dropped support for 32-bit architectures, we didn't actively remove
the code to drop support. As we no longer want to support 32-bit word
sizes with dqlite, as the number of jujud permutations explodes and
so it's best to completely remove the support.

The brings in the the juju/utils arch support, with the addition of
a UnsupportedArches field, which will ensure that we can at least
say what we use to support, but no longer do.

Additionally, I've dropped support for ppc and we explicitly callout
ppc64(le|el).
@SimonRichardson SimonRichardson added bug The PR addresses a bug 3.2 labels Apr 25, 2023
@SimonRichardson SimonRichardson self-assigned this Apr 25, 2023
{"ppc64le", "ppc64el"},
{"s390x", "s390x"},
{"riscv64", "riscv64"},
{"risc", "riscv64"},
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm unsure about this one, as we do move it to riscv64, I'm not a fan of dropping the word size, I think we should be a lot stricter.

raw string
arch string
}{
{"windows", "windows"},
Copy link
Member Author

Choose a reason for hiding this comment

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

I've just noticed, how is "windows" an arch?

Copy link
Member

Choose a reason for hiding this comment

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

Remove it - we no longer support windows charms anyway

@SimonRichardson
Copy link
Member Author

/build

@SimonRichardson SimonRichardson force-pushed the remove-32-bit-arches branch 2 times, most recently from 88a4885 to 08a7e6f Compare April 26, 2023 13:48
Copy link
Member

@hmlanigan hmlanigan left a comment

Choose a reason for hiding this comment

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

Should follow up and remove arch from utils, make it utils v4 so we don't accidentally start using it again. Especially with merges from 3.1 etc.

What are the odds there's a juju config running focal on i386 somewhere that might try to upgrade to juju 3.2? What happens if so?

core/arch/arches.go Outdated Show resolved Hide resolved
core/arch/arches.go Outdated Show resolved Hide resolved
apiserver/facades/client/application/deployrepository.go Outdated Show resolved Hide resolved
caas/kubernetes/provider/k8s.go Outdated Show resolved Hide resolved
raw string
arch string
}{
{"windows", "windows"},
Copy link
Member

Choose a reason for hiding this comment

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

Remove it - we no longer support windows charms anyway

provider/dummy/environs.go Outdated Show resolved Hide resolved
@SimonRichardson
Copy link
Member Author

/merge

@jujubot jujubot merged commit 7d8b745 into juju:3.2 Apr 28, 2023
17 checks passed
@SimonRichardson SimonRichardson deleted the remove-32-bit-arches branch April 28, 2023 10:43
@SimonRichardson
Copy link
Member Author

SimonRichardson commented Apr 28, 2023

@hmlanigan RE:

Should follow up and remove arch from utils, make it utils v4 so we don't accidentally start using it again

I can't land it because there is a ssh test race which I'm unable to fix atm juju/utils#330

@barrettj12 barrettj12 mentioned this pull request May 9, 2023
jujubot added a commit that referenced this pull request May 9, 2023
@barrettj12 barrettj12 mentioned this pull request May 10, 2023
jujubot added a commit that referenced this pull request May 10, 2023
@anvial anvial mentioned this pull request Oct 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The PR addresses a bug
Projects
None yet
4 participants