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

Revert "Merge pull request #1983 from Dudek-AMS/develop" #1995

Closed
wants to merge 1 commit into from
Closed

Revert "Merge pull request #1983 from Dudek-AMS/develop" #1995

wants to merge 1 commit into from

Conversation

Kenneth-Sills
Copy link

This reverts commit e05d705, reversing changes made to 59af6ad.

What does this PR do?

I spoke a bit about what I think happened here, but in short:

The two lines of link changes in #1983 caused a regression by updating the legacy repository installation codepath with links to the new, incompatible onedir file hierarchy.

  • Works:
    • http://repo.saltproject.io/py3/debian/11/amd64/latest/salt-archive-keyring.gpg
    • http://repo.saltproject.io/salt/py3/debian/11/amd64/SALT-PROJECT-GPG-PUBKEY-2023.gpg
  • Doesn't Work:
    • http://repo.saltproject.io/py3/debian11/amd64/latest/SALT-PROJECT-GPG-PUBKEY-2023.gpg

Subsequently, the release of v2024.04.03 has broken all the Debian 10/11 VM provisioning at my workplace (currently pinned to salt version 3004.2).

I've chosen to do a simple revert since the original PR changed code that was unrelated to the quoted issue. __install_saltstack_debian_repository was updated instead of __install_saltstack_debian_onedir_repository, but the former has no ability to generate the problematic https://repo.saltproject.io/salt/py3/debian/12/amd64/latest/salt-archive-keyring.gpg link they were referencing without a custom repo URL being passed in with -R.

What was more likely is that the latter function attempts to download from legacy before trying the onedir repository before continuing to the SALT-PROJECT-GPG-PUBKEY-2023.gpg case, and the author was just seeing that uncaptured output go to STDERR and assumed it was a failure.

This is supported since we never actually replicated their reported issue and they never provided evidence on the bootstrap process failing. In fact, right after the error line in their terminal we see "Hit: ...", indicating that execution continued. And looking at #1988 we can see exactly that, resulting in a successful bootstrapping. The issue there really being that the script should more clearly explain that it's a soft failure and that it's moving on to a different method OR send that initial command's STDERR to the void.

What issues does this PR fix or reference?

Resolves #1986 (closed, but not yet addressed).

Previous Behavior

All legacy repository (non-onedir) repository installations on Debian fail due to an invalid URL used to download the GPG key.

New Behavior

It works.

This does NOT address the issue described by the original PR or #1988. The STDERR line is still generated.

This reverts commit e05d705, reversing
changes made to 59af6ad, which had
caused a regression.
@Kenneth-Sills
Copy link
Author

Apologies, my original PR contained unsigned commits. I've rebased the branch with signed commits, so should be good now.

@twangboy twangboy requested a review from dmurphy18 April 9, 2024 17:18
@Kenneth-Sills
Copy link
Author

Bump, so this doesn't get lost.

@Kenneth-Sills
Copy link
Author

Kenneth-Sills commented Jul 12, 2024

@dmurphy18 This was not fixed by #1987. The offending line is still present. I tested and confirmed it still generates the same URL and, subsequently, the same error:

Outdated
 * ERROR: https://repo.saltproject.io/py3/debian/11/amd64/SALT-PROJECT-GPG-PUBKEY-2023.gpg failed to download to /tmp/salt-gpg-FgUAh7Nq.pub
 * ERROR: Failed to run install_debian_git_deps()!!!

Hm... Scratch that, that URL didn't look right (missing the /salt/ your changes added) so I threw together a brand new, from-scratch environment and pointed it to develop. That seems to work! 🎉

Closing this superseded PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] fails to install from master branch on Debian 11
1 participant