-
Notifications
You must be signed in to change notification settings - Fork 12
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
feat: move add-cluster.sh script logic into register-member command #53
Conversation
# Conflicts: # pkg/cmd/adm/register_member.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good, a few cosmetic comments and proposals in the test
Co-authored-by: Matous Jobanek <mjobanek@redhat.com>
Co-authored-by: Matous Jobanek <mjobanek@redhat.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job! Just a very minor request for a clarifying comment :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, great job 👍 🚀 🥇
Co-authored-by: Matous Jobanek <mjobanek@redhat.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #53 +/- ##
==========================================
+ Coverage 68.63% 69.45% +0.81%
==========================================
Files 43 43
Lines 2468 2544 +76
==========================================
+ Hits 1694 1767 +73
- Misses 584 587 +3
Partials 190 190
|
Jira: https://issues.redhat.com/browse/KUBESAW-27
Update the
ksctl register-member
command so that it doesn't rely anymore on the execution of theadd-cluster.sh
script. The logic from the bash script is now implemented in Golang. The original idea was to convert only since the creation of the Secret containing the kubeconfig, but I found it easier to convert the entire logic and remove the execution of the bash script. This simplifies the code and the tests.The creation of the toolchaincluster is just temporary until we implement https://issues.redhat.com/browse/KUBESAW-44, which will move the creation of TC in the toolchaincluster_resource controller and will remove it from here.