tree: add basic HACKING.md with section for MCD

[jlebon]

Closes: #8

[ashcrow]

This looks to be still accurate even after e155824. The items at the manifest dir level no longer need a namespace, but items within subdirectories look like they still require it.

[ashcrow]

@abhinavdahiya can you double check that?

[jlebon]

Yeah, that commit just removes it for the custom resources since the CRDs are now cluster scoped.

[abhinavdahiya]

WDYT if we copy valid mcd hacking manifests into examples/** directory. don't want people changing manifests directory.

[ashcrow]

@abhinavdahiya as in providing a second set in the repo for people to modify? I worry that would cause maintenance burden when a change happened in in the manifests. However, if the instructions note to copy the manifests directory to $LOCATION before editing that would make sense to me.

[abhinavdahiya]

👍 on copy to another location.

[jlebon]

I left this out for now. It doesn't seem like a big deal either way? The diff will be tracked by git status, and plus not copying makes it easier to iterate on other fields in the manifests.

[wking]

Hooray docs :). Just a few minor suggestions...

[wking]

nit: can you use:

```sh

here? More details on console vs sh for syntax highlighting in openshift/installer#27.

[wking]

nit: drop "file" from this line? Folks should be able to figure out that /opt/tectonic/node-annotations.json is a file and /etc/machine-config-daemon/ is a directory on their own ;).

[wking]

In fact, you can drop the "Copy /opt/tectonic/node-annotations.json to..." line entirely and use:

2. Run:

    ```sh
    sed s/node-configuration.v1.coreos.com/machineconfiguration.openshift.io/g /opt/tectonic/node-annotations.json >/etc/machine-config-daemon/node-annotations.json
    ```

Then the target file is never on-disk without the substitution.

The hanging indents in my suggestion will also make the command being executed part of this list entry. With your current form, it leaks out:

$ curl -s https://github.com/openshift/machine-config-operator/blob/2da5a0069cd9c72606f9a4ec0035cfeefa1b60d2/HACKING.md | grep -3 'sed -i s/node-'
<li>Create a cluster (e.g. using <a href="https://github.com/openshift/installer/blob/master/Documentation/dev/libvirt-howto.md">libvirt</a>)</li>
<li>Copy <code>/opt/tectonic/node-annotations.json</code> to <code>/etc/machine-config-daemon/</code> file on each node and pass it over this substitution:</li>
</ol>
<pre><code>sed -i s/node-configuration.v1.coreos.com/machineconfiguration.openshift.io/g /etc/machine-config-daemon/node-annotations.json
</code></pre>
<p>The install process will take care of this eventually.</p>
<ol start="3">

[wking]

nit: I'd prefer:

```sh

here too. And also four-space hanging indents for this shell block to keep it under the <li> for point four.

[sdemos]

last I checked, the installer still creates the old tectonic-node-agent daemonset on rhcos nodes. seems like it could screw up testing if we have something that doesn't understand what system it's operating on trying to stomp on stuff. maybe we should have a step for deleting those resources?

[jlebon]

OK, updated for comments!

last I checked, the installer still creates the old tectonic-node-agent daemonset on rhcos nodes

Ahh interesting. Is that still the case in git master of the installer? git grep node_agent (or node-agent) doesn't find anything.

[sdemos]

yeah, it's still true. it's created by the tectonic-utility-operator (https://github.com/coreos-inc/tectonic-operators/tree/master/operator/utility, deployed with this manifest in the installer).

It's probably still a good idea to get rid of it to make sure that there aren't issues, but it occurred to me later that they look for their configurations in different locations, so it will probably just sit there and do nothing since there is nothing to trigger it.

[sdemos]

also, I confirmed with the installer team that that utility operator shouldn't recreate the deployments for the node agent if the deployments are deleted manually, so it shouldn't be necessary to kill the utility operator itself (I'll probably kill it on my test cluster anyway though).

[jlebon]

Thanks! Now updated to also delete the legacy node agent. ✔️

[ashcrow]

LGTM

[jlebon]

/lgtm

[openshift-ci-robot]

@jlebon: you cannot LGTM your own PR.

Details

In response to this:

/lgtm

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.

[jlebon]

Thanks for confirming @openshift-ci-robot. Can someone get this merged?

[ashcrow]

/lgtm

[ashcrow]

/cc @abhinavdahiya

[ashcrow]

/cc @crawford

[abhinavdahiya]

/lgtm

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya, ashcrow, jlebon

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [abhinavdahiya]

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