vsphere ipi: set hostname using vmtoolsd and VM extra config #1579

jcpowermac · 2020-03-24T22:01:58Z

This commit adds two new files:

A systemd unit that runs a shell script
A shell script that runs vmtoolsd to retrieve guestinfo.hostname
from vm guest. Then uses hostnamectl to set hostname.

abhinavdahiya · 2020-03-25T22:35:16Z

templates/common/vsphere/files/vsphere-hostname.yaml

+    #!/usr/bin/env bash
+    set -e
+
+    if hostname=$(/bin/vmtoolsd --cmd 'info-get guestinfo.hostname'); then


is this binary present in the RHCOS system today? should we inform RHCOS people that please don't remove this binary in later versions as we depend on it.

Yeah its in RHCOS today. I tested out all the commands prior to scripting and using systemd to execute.

It's not in FCOS though and we do want to eventually containerize it (once vmware stops fighting that).

@cgwalters I have an example of running open-vm-tools in a container on fcos. Using @dav1x blog post as an example: https://developers.redhat.com/blog/2017/03/23/containerizing-open-vm-tools-part-1-the-dockerfile-and-constructing-a-systemd-unit-file/

It would be interesting if I could query guestinfo by using an exec but I have yet to try.

https://gist.github.com/jcpowermac/23ea90e549c02b43758662f90d83a9f1

iirc we layer vmtoolsd into OKD's machine-os-content, so it'll be on master/worker nodes after bootstrapping the cluster.
Is this script and service also present on bootstrap hosts?

@LorbusChris nope I didn't add it to bootstrap. bootstrap is fine as localhost.

I had to do something similar to this when I converted the container to runc. The exec should work fine:

I changed the systemd unit file to call this shell script instead.

#!/bin/sh if [ -f /tmp/open-vm-tools-bash-env ] then source /tmp/open-vm-tools-bash-env fi COMMAND=/usr/local/bin/vmware-toolbox-cmd if [ ! -e $COMMAND ] then echo 'runc exec -t open-vm-tools vmware-toolbox-cmd "$@"' > /usr/local/bin/vmware-toolbox-cmd chmod +x /usr/local/bin/vmware-toolbox-cmd fi exec /usr/bin/VGAuthService -s & exec /usr/bin/vmtoolsd

abhinavdahiya · 2020-03-25T22:37:17Z

templates/common/vsphere/files/vsphere-hostname.yaml

+    set -e
+
+    if hostname=$(/bin/vmtoolsd --cmd 'info-get guestinfo.hostname'); then
+        /usr/bin/hostnamectl --transient --static set-hostname ${hostname}


hostnamectl set-hostname --help hostnamectl [OPTIONS...] COMMAND ... Query or change system hostname. -h --help Show this help --version Show package version --no-ask-password Do not prompt for password -H --host=[USER@]HOST Operate on remote host -M --machine=CONTAINER Operate on local container --transient Only set transient hostname --static Only set static hostname --pretty Only set pretty hostname

is providing both transient and static valid option?

yep...tested and reviewed man page.

abhinavdahiya · 2020-03-25T22:37:46Z

/lgtm

runcom · 2020-03-26T17:50:28Z

/approve
/retest

openshift-bot · 2020-03-26T19:23:51Z

/retest

Please review the full test history for this PR and help us cut down flakes.

cgwalters · 2020-03-26T19:25:58Z

OK but...we've had a whole lot of extensive discussions about vmware and networking (which includes hostname settings for static IP cases).

See for example coreos/fedora-coreos-tracker#427 and coreos/ignition-dracut#156 as well as coreos/afterburn#379

A core problem with the hostname is that it's messy who owns it and sets it.

One option is to ensure we only run this if a hostname was not otherwise set (i.e. only set if it's localhost).

Does setting this value require the admin to do something specifically, or the installer will be setting it?

openshift-bot · 2020-03-26T19:49:35Z

/retest

Please review the full test history for this PR and help us cut down flakes.

jcpowermac · 2020-03-26T20:05:28Z

Does setting this value require the admin to do something specifically, or the installer will be setting it?

For masters its the installer via terraform for workers its MAO

openshift-bot · 2020-03-26T20:41:35Z

/retest

Please review the full test history for this PR and help us cut down flakes.

This commit adds two new files: - A systemd unit that runs a shell script - A shell script that runs vmtoolsd to retrieve `guestinfo.hostname` from vm guest. Then uses hostnamectl to set hostname.

jcpowermac · 2020-03-27T13:42:29Z

@abhinavdahiya I added a check for localhost when you have a moment can you please /lgtm.

cgwalters · 2020-03-27T14:32:57Z

Since this is IPI, the installer today can just update the Ignition config for each host to include /etc/hostname - this is a supported cross-platform way to set the hostname. I'd strongly prefer this unless there's a good reason the installer can't do this.

cgwalters · 2020-03-27T14:33:10Z

/hold
per above

jcpowermac · 2020-03-27T14:36:05Z

Since this is IPI, the installer today can just update the Ignition config for each host to include /etc/hostname - this is a supported cross-platform way to set the hostname. I'd strongly prefer this unless there's a good reason the installer can't do this.

MAO creates workers the installer does not.

cgwalters · 2020-03-27T14:42:34Z

MAO creates workers the installer does not.

Right, though the installer generates the user data. But...it's true today we don't support having dynamic userdata from the MAO (which would really then directly boil down to machine-specific machineconfigs since the userdata just points to the MCS, which we do want for the MCO, but that problem then goes into identifying machines...)

OK so I think where we really want this code then is as part of
https://github.com/coreos/afterburn/
which already today supports hostname across various providers but it looks like we don't have VMWare yet. I think @lucab is working on it?

One continual problem is people add random "grab bag" things to the MCO that might otherwise go into other CoreOS components I think because it's a repository conveniently integrated with OpenShift CI whereas patching other things goes into the world of RPM pain.

OK though short term,
/hold cancel
because we can at least drop this code once the afterburn code lands.

enxebre · 2020-03-27T14:44:32Z

cc @alexander-demichev

jcpowermac · 2020-03-27T14:47:14Z

OK so I think where we really want this code then is as part of
https://github.com/coreos/afterburn/
which already today supports hostname across various providers but it looks like we don't have VMWare yet. I think @lucab is working on it?

Completely agree. When that is ready please let me know and we can switch it out.

ashcrow · 2020-03-27T14:52:28Z

OK so I think where we really want this code then is as part of
https://github.com/coreos/afterburn/
which already today supports hostname across various providers but it looks like we don't have VMWare yet. I think @lucab is working on it?

I agree 👍 I think afterburn makes sense or maybe nm-cloud-setup per previous conversations if this is a more general need.

cgwalters · 2020-03-27T14:55:35Z

xref https://github.com/coreos/afterburn/pull/379/files#r399323764

abhinavdahiya · 2020-03-27T14:59:22Z

/lgtm

openshift-ci-robot · 2020-03-27T15:00:44Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya, jcpowermac, runcom

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~~ [runcom]

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

openshift-bot · 2020-03-27T15:06:33Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-03-27T15:19:34Z

/retest

Please review the full test history for this PR and help us cut down flakes.

darkmuggle · 2020-03-27T15:39:18Z

To echo what @cgwalters said -- the way hostnames are set in Linux is really messing and its a landmine. If we have NM, Ignition (via a files stanza for /etc/hostname), Afterburn and the MCO, the debug burden starts to grow. Reducing the number of things that can act on the hostname is to our advantage, IMHO.

openshift-bot · 2020-03-27T16:50:36Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2020-03-27T18:34:35Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-ci-robot · 2020-03-27T20:34:26Z

@jcpowermac: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/e2e-aws-scaleup-rhel7	`41d45f0`	link	`/test e2e-aws-scaleup-rhel7`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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.

lucab · 2020-03-31T11:26:01Z

(Sorry for the delay, I was on PTO)

I'm unfortunately missing most context about this and there are no references in PR/commit, can anybody provide that?

In particular, before giving any meaningful directions, these are my doubts:

is this meant for either nodes with DHCP, or with static networking, or both?
if this is for nodes with DHCP, why does hostname Option 12 not work? If both are provided, which one is intended to be the valid one?
if this is for nodes with static networking, why does hostname kargs setting not work? If both are provided, which one is intended to be the valid one?
if the Ignition config specifies some /etc/hostname value, which setting is intended to be the valid one?
where does guestinfo.hostname come from? Is it something we just invented for OCP?
is the guestinfo allowed to change while a VM is running?
why is MCO a relevant place for this?

In general I agree with all the feedbacks above, this is troublesome because 1) it bypasses the (already too many) possible owners of hostname logic with a random script in an unrelated OCP repo 2) it also reverts the "Ignition as a single source of truth" for file config.

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 24, 2020

openshift-ci-robot requested review from cgwalters and yuqi-zhang March 24, 2020 22:02

jcpowermac mentioned this pull request Mar 24, 2020

vsphere ipi: set vm name to extra config - guestinfo.hostname openshift/installer#3342

Merged

jcpowermac changed the title ~~[wip] vsphere ipi: set hostname using vmtoolsd and VM extra config~~ vsphere ipi: set hostname using vmtoolsd and VM extra config Mar 25, 2020

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 Mar 25, 2020

abhinavdahiya reviewed Mar 25, 2020

View reviewed changes

openshift-ci-robot assigned abhinavdahiya Mar 25, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 25, 2020

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 26, 2020

vsphere ipi: set hostname using vmtoolsd and VM extra config

41d45f0

This commit adds two new files: - A systemd unit that runs a shell script - A shell script that runs vmtoolsd to retrieve `guestinfo.hostname` from vm guest. Then uses hostnamectl to set hostname.

jcpowermac force-pushed the vsphere_ipi_hostname branch from f2c656d to 41d45f0 Compare March 26, 2020 20:42

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Mar 26, 2020

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 27, 2020

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 27, 2020

cgwalters mentioned this pull request Mar 27, 2020

[WIP] providers/vmware: add an experimental rd-net-kargs coreos/afterburn#379

Closed

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 27, 2020

openshift-merge-robot merged commit 68bf465 into openshift:master Mar 27, 2020

vsphere ipi: set hostname using vmtoolsd and VM extra config #1579

vsphere ipi: set hostname using vmtoolsd and VM extra config #1579

Conversation

jcpowermac commented Mar 24, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LorbusChris Mar 27, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abhinavdahiya commented Mar 25, 2020

runcom commented Mar 26, 2020

openshift-bot commented Mar 26, 2020

cgwalters commented Mar 26, 2020

openshift-bot commented Mar 26, 2020

jcpowermac commented Mar 26, 2020 • edited

openshift-bot commented Mar 26, 2020

jcpowermac commented Mar 27, 2020

cgwalters commented Mar 27, 2020

cgwalters commented Mar 27, 2020

jcpowermac commented Mar 27, 2020

cgwalters commented Mar 27, 2020

enxebre commented Mar 27, 2020

jcpowermac commented Mar 27, 2020

ashcrow commented Mar 27, 2020

cgwalters commented Mar 27, 2020

abhinavdahiya commented Mar 27, 2020

openshift-ci-robot commented Mar 27, 2020

openshift-bot commented Mar 27, 2020

openshift-bot commented Mar 27, 2020

darkmuggle commented Mar 27, 2020

openshift-bot commented Mar 27, 2020

openshift-bot commented Mar 27, 2020

openshift-ci-robot commented Mar 27, 2020

lucab commented Mar 31, 2020 • edited

LorbusChris Mar 27, 2020 •

edited

jcpowermac commented Mar 26, 2020 •

edited

lucab commented Mar 31, 2020 •

edited