Skip to content

Add noderef controller documentation to gitbook #1066

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

Merged

Conversation

vincepri
Copy link
Member

Signed-off-by: Vince Prignano [email protected]

What this PR does / why we need it:
This PR adds noderef controller docs to gitbook

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

Release note:


@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vincepri

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

The pull request process is described here

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 25, 2019
@k8s-ci-robot k8s-ci-robot requested review from detiber and justinsb June 25, 2019 01:57
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 25, 2019
@vincepri
Copy link
Member Author

/assign @davidewatson

@k8s-ci-robot k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jun 25, 2019
@@ -1,5 +1,8 @@
# Node Controller

> NOTE: This controller is going to be deprecated in v1alpha2,
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be ok to remove the node controller now, instead of saying it's deprecated?

Copy link
Member

Choose a reason for hiding this comment

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

Probably not, I know for example that the vSphere provider was relying on it at one point.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed, we should remove the node controller in v1alpha2. It's actually already part of #1051

Copy link
Member

Choose a reason for hiding this comment

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

It might be better to actually call this out as deprecated now, and state that it will be removed in v1alpha2, otherwise some may interpret this as it will still exist in a deprecated state for v1alpha2.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, just reworded

Copy link
Contributor

Choose a reason for hiding this comment

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

Does it need to remain in master now, though?

Copy link
Member Author

Choose a reason for hiding this comment

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

@davidewatson will probably have a better answer, but I think that the gitbook is served from master

Copy link
Contributor

Choose a reason for hiding this comment

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

@vincepri is correct, the book is served from master. To see a preview you can click the details of the deploy/netlify stage...


Infrastructure providers can opt-in to use this controller by providing a KubeConfig as a Kubernetes Secret.
The secret must be stored in the namespace the Cluster lives in and named as `<cluster-name>-kubeconfig`,
the data should only contain a single key called `value` and its data should be a valid Kubernetes KubeConfig.
Copy link
Contributor

Choose a reason for hiding this comment

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

@chuckha looks like we're using kubeconfig in the upgrade tool but CAPI is expecting value - I guess we should switch the upgrade tool?

@vincepri vincepri force-pushed the noderef-controller-docs branch from 2632924 to 5c6e1bb Compare June 25, 2019 15:14
@vincepri vincepri force-pushed the noderef-controller-docs branch from 5c6e1bb to 758f856 Compare June 25, 2019 15:15
@vincepri
Copy link
Member Author

/test pull-cluster-api-test

@davidewatson
Copy link
Contributor

#1052 has merged...
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 26, 2019
@k8s-ci-robot k8s-ci-robot merged commit 831e2cc into kubernetes-sigs:master Jun 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants