Skip to content

Update discovery installers to work with zypper#29360

Merged
lxea merged 1 commit intomasterfrom
lxea/zypper-scripts
Aug 24, 2023
Merged

Update discovery installers to work with zypper#29360
lxea merged 1 commit intomasterfrom
lxea/zypper-scripts

Conversation

@lxea
Copy link
Copy Markdown
Contributor

@lxea lxea commented Jul 20, 2023

This updates the automatic server discovery scripts to work with zypper

As it stands it hard-codes that its using the RHEL v8 teleport RPM repo, though I'm not sure if this is the best idea, and if we should instead host suse specific RPMs

Part of #29267

Comment thread api/types/installers/agentless-installer.sh.tmpl Outdated
Copy link
Copy Markdown
Collaborator

@r0mant r0mant left a comment

Choose a reason for hiding this comment

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

Can you update auto-upgrades and Discover install scripts as well?

Comment thread api/types/installers/agentless-installer.sh.tmpl Outdated
@r0mant r0mant requested a review from fheinecke July 20, 2023 18:50
@lxea
Copy link
Copy Markdown
Contributor Author

lxea commented Jul 24, 2023

Can you update auto-upgrades and Discover install scripts as well?

The discovery scripts are the ones updated here, are there other ones?

The auto-upgrades script is in e so i'll do that in a separate PR

Comment thread lib/web/scripts/node-join/install.sh Outdated
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Is there a reason we're using $OSTYPE rather than checking against uname ? It seems more portable and works on macos too.

opensuse defines OSTYPE as linux, ubuntu defines it as linux-gnu and alpine (for example) doesn't define it, where as uname is just Linux on all of them

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍 I'm happy with this as is - especially since thats currently how it is - but it'd be nice to see us use uname instead.

@lxea lxea force-pushed the lxea/zypper-scripts branch from 71361c6 to 160bfed Compare July 24, 2023 14:51
@r0mant
Copy link
Copy Markdown
Collaborator

r0mant commented Jul 24, 2023

@lxea By Discover I meant the node-join script that users download via Teleport Discover interface (when you click on add a server resource for example): https://github.com/gravitational/teleport/blob/master/lib/web/scripts/node-join/install.sh.

Copy link
Copy Markdown
Contributor

@ibeckermayer ibeckermayer left a comment

Choose a reason for hiding this comment

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

LGTM in terms of bash script fundamentals, the details are beyond my understanding

Comment thread lib/web/scripts/node-join/install.sh Outdated
@r0mant
Copy link
Copy Markdown
Collaborator

r0mant commented Jul 25, 2023

@lxea Let's hold this off until @fheinecke adds the SUSE channel so we can remove RHEL version hardcodes from the scripts.

@r0mant
Copy link
Copy Markdown
Collaborator

r0mant commented Aug 8, 2023

@fheinecke IIRC you set up the zypper repository and was going to configure a separate DNS name for it. Is it done now, can we update this PR?

@fheinecke
Copy link
Copy Markdown
Contributor

@r0mant Yes it is. I haven't had a chance to file doc PRs for the changes but an install example is available in our smoke tests here.

Note that prior to these two lines the user will need to run sudo rpm --import "https://zypper.releases.teleport.dev/gpg" to import the signing key.

@lxea lxea force-pushed the lxea/zypper-scripts branch 3 times, most recently from ab19082 to ea53313 Compare August 11, 2023 10:31
Copy link
Copy Markdown
Collaborator

@r0mant r0mant left a comment

Choose a reason for hiding this comment

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

@lxea Have you tested these changes for all 3 use-cases?

  • Automatic upgrade script
  • Discover install script
  • Auto-discovery install script

manually import the gpg key if we're on sles 12

Update the node-join installer

untabify
@lxea lxea force-pushed the lxea/zypper-scripts branch from ea53313 to fa78e95 Compare August 18, 2023 10:21
@lxea lxea removed the do-not-merge label Aug 18, 2023
@lxea
Copy link
Copy Markdown
Contributor Author

lxea commented Aug 18, 2023

@lxea Have you tested these changes for all 3 use-cases?

* [x]  Automatic upgrade script

* [x]  Discover install script

* [x]  Auto-discovery install script

Yeah, all of these appear to be working, tried on a fresh AWS SLES instance

Comment thread lib/web/scripts/node-join/install.sh Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍 I'm happy with this as is - especially since thats currently how it is - but it'd be nice to see us use uname instead.

@lxea lxea added this pull request to the merge queue Aug 24, 2023
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 24, 2023
@lxea lxea added this pull request to the merge queue Aug 24, 2023
Merged via the queue into master with commit 1294516 Aug 24, 2023
@lxea lxea deleted the lxea/zypper-scripts branch August 24, 2023 15:59
@lxea lxea restored the lxea/zypper-scripts branch September 5, 2023 13:01
@public-teleport-github-review-bot
Copy link
Copy Markdown

@lxea See the table below for backport results.

Branch Result
branch/v13 Create PR

@lxea lxea deleted the lxea/zypper-scripts branch September 5, 2023 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants