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

Final Neon Support Fix #1424

Merged
merged 25 commits into from
Feb 4, 2020
Merged

Final Neon Support Fix #1424

merged 25 commits into from
Feb 4, 2020

Conversation

s0undt3ch
Copy link
Contributor

@s0undt3ch s0undt3ch commented Feb 3, 2020

What does this PR do?

See title.
Additionally:

  • Drop support for Fedora < 30
  • Drop support for Debian < 8
  • Adds preliminary support for Fedora 32, although disabled since it defaults to Py3.8 which salt doesn't yet support
  • Locks kitchen config for Arch Linux to python 2 since Arch is also on Py3.8

What issues does this PR fix or reference?

Closes #1395
Closes #1400
Closes #1423
Closes #1381

@s0undt3ch s0undt3ch requested a review from Ch3LL February 3, 2020 18:45
@myii
Copy link
Contributor

myii commented Feb 3, 2020

@s0undt3ch As we discussed in #formulas a couple of days back, the fedora-31 image build that was failing then is now passing. To be completely sure, I'm rerunning all of the jobs again anyway:

@s0undt3ch
Copy link
Contributor Author

@myii Thank You! Please report back any problems.

@s0undt3ch
Copy link
Contributor Author

@vutny see anything requiring changes?

Copy link
Contributor

@vutny vutny left a comment

Choose a reason for hiding this comment

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

Wow, this PR is huge!
I was unable to spot anything critical, and the Drone looks happy.
Well done, @s0undt3ch . Thanks.

@myii
Copy link
Contributor

myii commented Feb 4, 2020

@s0undt3ch Just one note for you, which appears to be a regression:

These were all passing before this set of changes (not just this PR but the last batch of development). It no longer allows us install the latest master on top of an existing pre-salted image.

However, I wouldn't like to see this PR blocked, though -- the current bootstrap script needs to be fixed since it's broken. Hopefully this can be addressed in another PR.

@myii
Copy link
Contributor

myii commented Feb 4, 2020

@s0undt3ch That last link was the wrong one. I've redone the comparison, before and after:

All of the above are failures to build or find pycrypto.

@myii
Copy link
Contributor

myii commented Feb 4, 2020

@s0undt3ch Just had a bit more time to test and I've established that the pycrypto regression was introduced by this commit: 39ed3f8.

@s0undt3ch
Copy link
Contributor Author

The upcoming Neon release, salt's master branch has now defaulted to setuptools.
With that change, and because we hit an issue when testing the package build process, we reverted back to using pycrypto, for which there isn't a wheel available.
As such, we must build it from source, which seems to work (nevermind the opensuse, that a bad github actions workflow issue).

This is the whole reason why git bootstraps post Neon(which include Salt's master branch) are now done without system packages and only using pypi packages.
When I get some more time, I'll look at your CI pipeline to find out why its failing.

@myii
Copy link
Contributor

myii commented Feb 4, 2020

@s0undt3ch OK, thanks for the explanation. We'll just avoid using the bootstrap on top of the pre-salted images for the time being.

@s0undt3ch
Copy link
Contributor Author

By pre-salted you mean salt has been installed before and it's not the master branch, correct?

@myii
Copy link
Contributor

myii commented Feb 4, 2020

We're building pre-salted images to use with SaltStack Formulas, for numerous platforms and all supported releases of Salt + master. This is the repo:

To ensure our master testing is the very latest, we use the bootstrap on top. We'll just avoid that going forward; we've got a Travis cron job that runs every week anyway, so it will be pretty recent in any case.

@s0undt3ch
Copy link
Contributor Author

Ok. Thanks for all your help!

@myii
Copy link
Contributor

myii commented Feb 4, 2020

You're welcome. Any idea when this PR will be merged?

@s0undt3ch
Copy link
Contributor Author

As soon as drone passes, its going to be merged and a sable release will be done right after.

@myii
Copy link
Contributor

myii commented Feb 4, 2020

Excellent, appreciate all of the work you're putting in, @s0undt3ch.

@s0undt3ch s0undt3ch merged commit 5892d7d into saltstack:develop Feb 4, 2020
myii pushed a commit to myii/ssf-formula that referenced this pull request Feb 5, 2020
# [1.81.0](v1.80.0...v1.81.0) (2020-02-05)

### Features

* **amazonlinux:** update for `1` & `2` and remove temporary `develop` ([ce5e13a](ce5e13a))
* **kitchen:** avoid using bootstrap for `master` instances ([16de460](16de460)), closes [/github.com/saltstack/salt-bootstrap/pull/1424#issuecomment-581997903](https://github.com//github.com/saltstack/salt-bootstrap/pull/1424/issues/issuecomment-581997903)
vernondcole added a commit to vernondcole/salt-bootstrap that referenced this pull request Feb 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants