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

Support out-of-cluster installation #82

Merged

Conversation

salasberryfin
Copy link
Contributor

@salasberryfin salasberryfin commented Aug 16, 2023

What this PR does / why we need it:

This is intended to allow a Rancher operator to choose between deploying rancher-turtles in the same or a different cluster to Rancher Manager. The current implementation of rancher-turtles defaults to an in-cluster configuration with both Rancher Manager and rancher-turtles supposed to be installed together in the same cluster.

The initial idea is to create the client based on whether a flag with a path to a kubeconfig file is passed. When this parameter is used, rancher-turtles will load the REST Config from this file and create a client for an out-of-cluster installation. If it is not, it defaults to the current in-cluster behavior.

Issue #19

Special notes for your reviewer:

Checklist:

  • squashed commits into logical changes
  • includes documentation
  • adds unit tests
  • adds or updates e2e tests

@salasberryfin salasberryfin added the kind/enhancement Categorizes issue or PR as related to a new feature. label Aug 16, 2023
@salasberryfin salasberryfin force-pushed the support-out-of-cluster-installation branch 3 times, most recently from 2fb60d9 to ec9adf6 Compare August 16, 2023 14:25
main.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
@salasberryfin salasberryfin changed the title WIP: Support out-of-cluster installation Support out-of-cluster installation Aug 22, 2023
@salasberryfin salasberryfin marked this pull request as ready for review August 22, 2023 07:48
@salasberryfin salasberryfin requested a review from a team as a code owner August 22, 2023 07:48
internal/controllers/import_controller.go Outdated Show resolved Hide resolved
main.go Show resolved Hide resolved
@salasberryfin salasberryfin force-pushed the support-out-of-cluster-installation branch from ef8146a to 57e0a17 Compare August 24, 2023 07:36
Copy link
Contributor

@furkatgofurov7 furkatgofurov7 left a comment

Choose a reason for hiding this comment

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

While this is clear to the internal team, it would be nice to have a small document describing what is (out-of/in)-cluster support, the difference between them and why it is needed.

Please disregard if we have a separate issue to document it in rancher-turtles-docs

main.go Outdated Show resolved Hide resolved
@Danil-Grigorev
Copy link
Contributor

Same thought as @furkatgofurov7, but additionally we need to have some clear process to pass the kubeconfig to the pod, which could be documented in rancher-turtles-docs + not necessary in the same PR but would prefer to have e2e test coverage here to be sure.

@richardcase
Copy link
Contributor

@furkatgofurov7 @Danil-Grigorev @salasberryfin - i created #94 for us to create an ADR for this.

@furkatgofurov7
Copy link
Contributor

@furkatgofurov7 @Danil-Grigorev @salasberryfin - i created #94 for us to create an ADR for this.

Except this, I have nothing major to not merge it

@salasberryfin
Copy link
Contributor Author

Thanks for the reviews @richardcase @Danil-Grigorev @furkatgofurov7. I'll be taking on the ADR proposal to have this well documented.

@salasberryfin salasberryfin force-pushed the support-out-of-cluster-installation branch 2 times, most recently from dc071c0 to 81161b8 Compare August 31, 2023 11:26
@salasberryfin
Copy link
Contributor Author

I addressed the latest comments and created a PR for the ADR #95.

richardcase
richardcase previously approved these changes Aug 31, 2023
Copy link
Contributor

@richardcase richardcase left a comment

Choose a reason for hiding this comment

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

This looks good to me, thanks @salasberryfin

We will need to create a documentation issue (in racher-turtles-docs) to cover:

  • Handling the RBAC, service account etc
  • How to create the secret and mount it

@salasberryfin
Copy link
Contributor Author

I created rancher/turtles-docs#13 to track this.

@furkatgofurov7
Copy link
Contributor

@salasberryfin can we rebase it?

@salasberryfin
Copy link
Contributor Author

The changes are now up to date after rebase, please take a look.

Copy link
Contributor

@furkatgofurov7 furkatgofurov7 left a comment

Choose a reason for hiding this comment

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

Thanks @salasberryfin!

@salasberryfin salasberryfin force-pushed the support-out-of-cluster-installation branch 2 times, most recently from 4ef9c9e to 5c7e2d1 Compare September 4, 2023 16:25
Signed-off-by: Carlos Salas <carlos.salas@suse.com>
Signed-off-by: Carlos Salas <carlos.salas@suse.com>
@salasberryfin salasberryfin force-pushed the support-out-of-cluster-installation branch from 5c7e2d1 to db01ba9 Compare September 4, 2023 16:31
@richardcase richardcase merged commit fc855ca into rancher:main Sep 5, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants