Skip to content

Allow MicroShift to join new worker nodes #471

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.

Sign up for GitHub

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

Jump to bottom
Closed
oglok wants to merge 9 commits into from
Closed

Allow MicroShift to join new worker nodes #471

oglok wants to merge 9 commits into from

Conversation

@oglok
Copy link
Contributor

@oglok oglok commented Nov 25, 2021 •
edited by mangelajo
Loading

Allow MicroShift to join new worker nodes, according to design here #498 (see individual commits for review),

  • 4523987 Add flags to allow TLS bootstrapping of nodes
  • 9d8e2ad Add bootstrap module and generate token file
  • 26f04a3 Add ClusterRoleBinding for bootstrap process
  • 1103d73 Generate bootstrap kubeconfig
  • 1fd2a3c Allow MicroShift to start node role standalone
  • 6c84f16 Apply CRB for bootstraping nodes
  • 1366626 Use bootstrap kubeconfig for kube-proxy
  • c2eacfd Use netcgo insted of netgo
  • 40bff39 Add vagrant env to test/devel/debug multi-worker

Related PRs: #499 , #500

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 25, 2021
@openshift-ci openshift-ci bot requested review from copejon and fzdarsky November 25, 2021 12:22
@oglok
Copy link
Contributor Author

oglok commented Nov 25, 2021

For some reason, the cluster role bindings added using bindata strategy are not being applied. That causes kubelet to:

E1125 16:20:51.785446   27509 server.go:294] "Failed to run kubelet" err="failed to run Kubelet: cannot create certificate signing request: certificatesigningrequests.certificates.k8s.io is forbidden: User \
"kubelet-bootstrap\" cannot create resource \"certificatesigningrequests\" in API group \"certificates.k8s.io\" at the cluster scope"

@oglok oglok mentioned this pull request Nov 29, 2021
Closed
@oglok
Copy link
Contributor Author

oglok commented Nov 29, 2021

Now the ClusterRoleBindings get properly applied.

@oglok
Copy link
Contributor Author

oglok commented Nov 29, 2021

https://asciinema.org/a/452317

@mangelajo mangelajo force-pushed the multi_node branch 2 times, most recently from e3aeaa1 to 88d042f Compare December 1, 2021 12:39
@mangelajo mangelajo mentioned this pull request Dec 2, 2021
Closed
@mangelajo mangelajo force-pushed the multi_node branch 2 times, most recently from c5ac2bf to a12ded3 Compare December 3, 2021 10:24
mangelajo
mangelajo reviewed Dec 10, 2021
View reviewed changes
mangelajo
mangelajo reviewed Dec 10, 2021
View reviewed changes
mangelajo
mangelajo reviewed Dec 10, 2021
View reviewed changes
mangelajo
mangelajo reviewed Dec 10, 2021
View reviewed changes
mangelajo
mangelajo reviewed Dec 10, 2021
View reviewed changes
oglok
oglok commented Dec 10, 2021
View reviewed changes
Copy link
Contributor Author

@oglok oglok left a comment

Choose a reason for hiding this comment

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

Good job! Some minor comments. Once the klog patch is out of the PR, we can remove the WIP so it can be reviewed.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 10, 2021

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please ask for approval from mangelajo after the PR has been reviewed.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mangelajo mangelajo removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 10, 2021
@mangelajo mangelajo changed the title [WIP] Allow MicroShift to join new worker nodes [Allow MicroShift to join new worker nodes Dec 10, 2021
@mangelajo mangelajo changed the title [Allow MicroShift to join new worker nodes Allow MicroShift to join new worker nodes Dec 10, 2021
@mangelajo mangelajo self-requested a review December 10, 2021 14:36
@fzdarsky
Copy link
Contributor

fzdarsky commented Dec 10, 2021

@oglok @mangelajo this is a pretty big PR and conflates many changes (introducing debug flags, formatting changes, config param changes, mDNS, multi-node, etc.) into one, which makes it hard to review. Can we split this into independent PRs?

@fzdarsky
Copy link
Contributor

fzdarsky commented Dec 10, 2021

Also, as this impacts the MicroShift architecture and UX, we'll need an RFE doc in /docs/design that describes the design trade-offs and proposed solution before that solution can be implemented.

@oglok
Copy link
Contributor Author

oglok commented Dec 10, 2021

/retest

@mangelajo
Copy link
Contributor

mangelajo commented Dec 10, 2021

@fzdarsky its broken into logical commits, we expected that would suffice for agility.

If we go the PR route this is going to be eternal.

I'm ok with extracting specific commits to individual PRs but I don't think is a good approach to split all since those are dependent (if we want this for 2021) I'm fine if we are ok with slipping off Quarter

@mangelajo
Copy link
Contributor

mangelajo commented Dec 10, 2021

In gerrit chained patches are easier but GitHub is a mess when you try to chain dependent PRs

@mangelajo
Copy link
Contributor

mangelajo commented Dec 10, 2021

I will split out some patches which can be out of the chain and topic of this PR without great issue, the only thing is that for this PR to be functional (or if somebody wants to test the PR) those other patches will need to be merged first

@mangelajo
Copy link
Contributor

mangelajo commented Dec 13, 2021

Extracted to separate PR: #497

oglok and others added 6 commits December 13, 2021 16:12
@oglok @mangelajo
Generate bootstrap kubeconfig
1103d73 
Signed-off-by: Ricardo Noriega <[email protected]>
@oglok @mangelajo
Allow MicroShift to start node role standalone
1fd2a3c 
  This patch only tackles the Kubelet configuration
  and not kube-proxy.

Signed-off-by: Ricardo Noriega <[email protected]>
@oglok @mangelajo
Apply CRB for bootstraping nodes
6c84f16 
Signed-off-by: Ricardo Noriega <[email protected]>
@oglok @mangelajo
Use bootstrap kubeconfig for kube-proxy
1366626 
Signed-off-by: Ricardo Noriega <[email protected]>
Signed-off-by: Miguel Angel Ajo <[email protected]>
@mangelajo
Use netcgo insted of netgo
c2eacfd 
netgo is the network library in go, which implements network functions
without using the basic linux libraries, but netgo does not implement
mDNS resolution, the system can be configured to do mDNS name
resolution.

This doesn't add new dynamic libraries simply uses the existing ones,
but stops compiling in the native-go network functions.

Signed-off-by: Miguel Angel Ajo <[email protected]>
@mangelajo
Add vagrant env to test/devel/debug multi-worker
40bff39 
Signed-off-by: Miguel Angel Ajo <[email protected]>
@mangelajo
Copy link
Contributor

mangelajo commented Dec 13, 2021

@oglok @fzdarsky I hope it looks better now, I'm going to review the RFE

@oglok
Copy link
Contributor Author

oglok commented Jan 18, 2022

/retest

2 similar comments
@oglok
Copy link
Contributor Author

oglok commented Feb 14, 2022

/retest

@copejon
Copy link
Contributor

copejon commented Feb 21, 2022

/retest

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 21, 2022

@oglok: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/periodics-images 40bff39 link true /test periodics-images
ci/prow/e2e-rpm-install 40bff39 link false /test e2e-rpm-install
ci/prow/e2e-openshift-conformance-sig-instrumentation 40bff39 link false /test e2e-openshift-conformance-sig-instrumentation
ci/prow/e2e-openshift-conformance-sig-network 40bff39 link false /test e2e-openshift-conformance-sig-network
ci/prow/e2e-openshift-conformance-sig-storage 40bff39 link false /test e2e-openshift-conformance-sig-storage
ci/prow/e2e-openshift-conformance-sig-cli 40bff39 link false /test e2e-openshift-conformance-sig-cli
ci/prow/e2e-openshift-conformance-sig-scheduling 40bff39 link false /test e2e-openshift-conformance-sig-scheduling
ci/prow/e2e-openshift-conformance-sig-apps 40bff39 link false /test e2e-openshift-conformance-sig-apps
ci/prow/test-srpm 40bff39 link true /test test-srpm
ci/prow/e2e-reboot 40bff39 link false /test e2e-reboot
ci/prow/e2e-openshift-conformance-sig-node 40bff39 link false /test e2e-openshift-conformance-sig-node
ci/prow/e2e-openshift-conformance-sig-api-machinery 40bff39 link false /test e2e-openshift-conformance-sig-api-machinery
ci/prow/e2e-openshift-conformance-sig-arch 40bff39 link false /test e2e-openshift-conformance-sig-arch
ci/prow/e2e-openshift-conformance-sig-auth 40bff39 link false /test e2e-openshift-conformance-sig-auth

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@openshift-bot
Copy link

openshift-bot commented May 23, 2022

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci openshift-ci bot added lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels May 23, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented May 23, 2022

@oglok: PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@oglok
Copy link
Contributor Author

oglok commented Jun 20, 2022

Closing for now until we decide the multi-node strategy. This is a good PoC for learning to build the right solution.

@oglok oglok closed this Jun 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@copejon copejon Awaiting requested review from copejon

@fzdarsky fzdarsky Awaiting requested review from fzdarsky

@cooktheryan cooktheryan Awaiting requested review from cooktheryan

@sallyom sallyom Awaiting requested review from sallyom

@mangelajo mangelajo Awaiting requested review from mangelajo

Assignees

@mangelajo mangelajo

@oglok oglok

Labels

lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@oglok @fzdarsky @mangelajo @copejon @openshift-bot