Skip to content

Document entry-point for user-provided custom MachineConfig #1331

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
Merged
merged 1 commit into from
Mar 4, 2019
Merged

Document entry-point for user-provided custom MachineConfig #1331

merged 1 commit into from
Mar 4, 2019

Conversation

abhinavdahiya
Copy link
Contributor

@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Feb 28, 2019
@wking
Copy link
Member

wking commented Feb 28, 2019

The troubleshooting commit seems orthogonal (and it's in #1329). Can we drop it here?

wking
wking reviewed Feb 28, 2019
View reviewed changes
wking
wking reviewed Feb 28, 2019
View reviewed changes
wking
wking reviewed Feb 28, 2019
View reviewed changes
wking
wking reviewed Feb 28, 2019
View reviewed changes
wking
wking reviewed Feb 28, 2019
View reviewed changes
wking
wking reviewed Feb 28, 2019
View reviewed changes
wking
wking reviewed Feb 28, 2019
View reviewed changes
wking
wking reviewed Feb 28, 2019
View reviewed changes
wking
wking reviewed Feb 28, 2019
View reviewed changes
wking
wking reviewed Feb 28, 2019
View reviewed changes
@abhinavdahiya abhinavdahiya changed the title WIP: Document entry-point for user-provided custom MachineConfig Document entry-point for user-provided custom MachineConfig Feb 28, 2019
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 28, 2019
wking
wking reviewed Feb 28, 2019
View reviewed changes
wking
wking reviewed Feb 28, 2019
View reviewed changes
@wking wking mentioned this pull request Feb 28, 2019
Merged
wking
wking reviewed Feb 28, 2019
View reviewed changes
kikisdeliveryservice
kikisdeliveryservice reviewed Feb 28, 2019
View reviewed changes
Copy link
Contributor

@kikisdeliveryservice kikisdeliveryservice left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Just a few thoughts. Docs are so helpful though! 👍

docs/user/customization.md
```

```console
$ oc --config no-taint-cluster/auth/kubeconfig get nodes -ojson | jq '.items[] | select(.metadata.labels."node-role.kubernetes.io/master" == "") | .spec.taints'
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a line about what this is doing (esp bc this is half hidden in my browser so the full command isn't visible without lots of scrolling).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

Choose a reason for hiding this comment

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

Perfecto!!

docs/user/customization.md
```

```console
oc --config no-taint-cluster/auth/kubeconfig get machineconfigs
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a line about what this is doing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

Choose a reason for hiding this comment

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

nice!

@abhinavdahiya abhinavdahiya force-pushed the docs-custom-mc branch 2 times, most recently from 44f0909 to 88d4374 Compare March 1, 2019 00:45
wking
wking reviewed Mar 1, 2019
View reviewed changes
docs/user/customization.md Outdated
@@ -64,4 +64,172 @@ An example `worker.ign` is shown below. It has been modified to increase the HTT
}
```

## Install Time Customization for Machine Configuration
Copy link
Member

Choose a reason for hiding this comment

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

Should this go under here, since that's the section about manifests? Or do you want to avoid going into detail there and maybe just link out to this new section?

@abhinavdahiya
Copy link
Contributor Author

ping @wking

wking
wking reviewed Mar 4, 2019
View reviewed changes
docs/user/customization.md Outdated
```

[default-kubelet-service]: https://github.com/openshift/machine-config-operator/blob/master/templates/master/01-master-kubelet/_base/units/kubelet.yaml
[ignition]: https://coreos.com/ignition/docs/latest/
Copy link
Member

Choose a reason for hiding this comment

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

I think these references should stay at the bottom of the file (e.g. the [ignition] reference is (also?) consumed by the next OS Customization (unvalidated) section.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved ignition by mistake. fixed

but looks like other sections have their own anchors https://github.com/openshift/installer/pull/1331/files#diff-ae4bcbdd69693a4977c2088180a61927R19 so keeping the rest as-is.

@abhinavdahiya
docs: add details on install time customization for machine configura…
0900249 
…tion

The docs include an example for removing `NoSchedule` taint on masters to allow for single master cluster. Although the changes described in the
example are not completely sufficient to achieve the single master cluster, as few operators specifcally set workloads to worker nodes.

I chose this example because the Code Ready Containers team will be using somehting similar to achieve single node libvirt cluster.
@wking
Copy link
Member

wking commented Mar 4, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 4, 2019
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya, wking

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:
  • OWNERS [abhinavdahiya,wking]

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

@openshift-merge-robot openshift-merge-robot merged commit 0ddac41 into openshift:master Mar 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wking wking wking left review comments

@aaronlevy aaronlevy Awaiting requested review from aaronlevy

@staebler staebler Awaiting requested review from staebler

+1 more reviewer

@kikisdeliveryservice kikisdeliveryservice kikisdeliveryservice left review comments

Reviewers whose approvals may not affect merge requirements
Assignees

@wking wking

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@abhinavdahiya @wking @openshift-ci-robot @kikisdeliveryservice @openshift-merge-robot