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

Auto Discover Server must not overwrite manual changes #44637

Merged
merged 4 commits into from
Jul 30, 2024

Conversation

marcoandredinis
Copy link
Contributor

@marcoandredinis marcoandredinis commented Jul 25, 2024

This PR changes the flow so that only discover managed installations might have their teleport.yaml configuration replaced.

Demo
Started by using a DiscoveryConfig which had a typo in the installer params.
As expected, the command was executed but the node couldn't join
Discover notice file was created:
image

The discover notice file was removed and the typo was fixed.
The script refused to re-configure teleport because the discover file was not present.
image

After fixing the typo and running the suggested command, and after a new iteration of SSM Run (discover flow), teleport was able to recover and join the cluster.

image

@marcoandredinis marcoandredinis added no-changelog Indicates that a PR does not require a changelog entry backport/branch/v16 labels Jul 25, 2024
@marcoandredinis marcoandredinis force-pushed the marco/autodiscover-server-prevent-config-overwrite branch 2 times, most recently from ac29b7f to 30de68a Compare July 25, 2024 17:12
This PR changes the flow so that only discover managed installations
might have their teleport.yaml configuration replaced.
@marcoandredinis marcoandredinis force-pushed the marco/autodiscover-server-prevent-config-overwrite branch from 30de68a to 28efbad Compare July 26, 2024 09:24
lib/srv/server/installer/autodiscover.go Outdated Show resolved Hide resolved
@marcoandredinis
Copy link
Contributor Author

@GavinFrazar can you please take a look?
I would like to have this one merged before the next release.
There's another PR that was merged and it might cause disruption if the AutoDiscover script runs against a manually managed teleport server.
#44282

lib/srv/server/installer/autodiscover.go Outdated Show resolved Hide resolved
@public-teleport-github-review-bot public-teleport-github-review-bot bot removed the request for review from GavinFrazar July 30, 2024 16:55
@marcoandredinis marcoandredinis added this pull request to the merge queue Jul 30, 2024
@GavinFrazar
Copy link
Contributor

@GavinFrazar can you please take a look? I would like to have this one merged before the next release. There's another PR that was merged and it might cause disruption if the AutoDiscover script runs against a manually managed teleport server. #44282

oops, sorry I don't know how I missed this last Friday

@GavinFrazar GavinFrazar removed this pull request from the merge queue due to a manual request Jul 30, 2024
lib/srv/server/installer/autodiscover.go Outdated Show resolved Hide resolved
@GavinFrazar GavinFrazar enabled auto-merge July 30, 2024 17:34
@GavinFrazar GavinFrazar added this pull request to the merge queue Jul 30, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 30, 2024
@r0mant r0mant added this pull request to the merge queue Jul 30, 2024
Merged via the queue into master with commit b19f54c Jul 30, 2024
38 checks passed
@r0mant r0mant deleted the marco/autodiscover-server-prevent-config-overwrite branch July 30, 2024 21:42
@public-teleport-github-review-bot

@marcoandredinis See the table below for backport results.

Branch Result
branch/v16 Create PR

michellescripts pushed a commit that referenced this pull request Jul 30, 2024
* Auto Discover Server must not overwrite manual changes

This PR changes the flow so that only discover managed installations
might have their teleport.yaml configuration replaced.

* Update lib/srv/server/installer/autodiscover.go

Co-authored-by: Ryan Clark <[email protected]>

* Update lib/srv/server/installer/autodiscover.go

Co-authored-by: Roman Tkachenko <[email protected]>

* Update lib/srv/server/installer/autodiscover.go

---------

Co-authored-by: Ryan Clark <[email protected]>
Co-authored-by: Roman Tkachenko <[email protected]>
Co-authored-by: Gavin Frazar <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/branch/v16 no-changelog Indicates that a PR does not require a changelog entry size/md
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants