Bug 2033953: Afterburn to try config-drive before Nova metadata #2903

pierreprinetti · 2022-01-10T14:41:41Z

With this change, the afterburn-hostname service uses the 'openstack'
provider, which fetches userdata from the config drive, and only uses
the Nova metadata service as a fallback. This is especially useful in
network topologies where Nova-metadata is not guaranteed to be reachable
(e.g. when using baremetal (Ironic) instances).

With this change, the afterburn-hostname service uses the 'openstack' provider, which fetches userdata from the config drive, and only uses the Nova metadata service as a fallback. This is especially useful in network topologies where Nova-metadata is not guaranteed to be reachable (e.g. when using baremetal (Ironic) instances). Co-authored-by: Martin André <m.andre@redhat.com> Co-authored-by: Pierre Prinetti <pierreprinetti@redhat.com>

openshift-ci · 2022-01-10T14:41:45Z

@pierreprinetti: This pull request references Bugzilla bug 2033953, which is invalid:

expected the bug to target the "4.10.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

Bug 2033953: Afterburn to try config-drive before Nova metadata

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.

pierreprinetti · 2022-01-10T14:42:42Z

/bugzilla refresh

openshift-ci · 2022-01-10T14:42:49Z

@pierreprinetti: This pull request references Bugzilla bug 2033953, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.10.0) matches configured target release for branch (4.10.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @eurijon

In response to this:

/bugzilla refresh

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.

openshift-ci · 2022-01-10T14:45:41Z

@pierreprinetti: This pull request references Bugzilla bug 2033953, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.10.0) matches configured target release for branch (4.10.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @eurijon

In response to this:

Bug 2033953: Afterburn to try config-drive before Nova metadata

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.

pierreprinetti · 2022-01-10T15:24:25Z

/cc mandre

cgwalters · 2022-01-10T22:39:11Z

This intersects a bit with a whole huge recent saga around hostnames; see https://bugzilla.redhat.com/show_bug.cgi?id=2008521#c40
and coreos/afterburn#509

A lot of confusion and overlap between "ownership" of the hostname. While this isn't new, the presence of this file is perpetuating a difference between FCOS and RHCOS behavior on OpenStack. (I think this is rooted in the uncertain availability of the metadata service across OpenStack deployments)

Another related topic; AIUI the reason for the current design is that availability of the config drive is racy. I don't know if that applies in Ironic or not.

pierreprinetti · 2022-01-11T08:37:30Z

@cgwalters
My intention here is to only make a tactic adjustment to the way a host fetches userdata: changing the behaviour from "fetch Nova metadata" to "fetch configdrive; if failed, fetch Nova metadata".

I couldn't find Afterburn providers' documentation, but here's the relevant code:

match provider {
        // ...
        "openstack" => openstack::try_config_drive_else_network(),
        "openstack-metadata" => box_result!(OpenstackProviderNetwork::try_new()?),
        // ...

Is this change having a larger blast radius than what I expect?

/retest-required

cgwalters · 2022-01-11T14:09:48Z

Yes,
/approve
since I think this is probably not going to break anything.

But one other question: what is the role of Ironic in OCP going forward? I thought the move to use the CoreOS "Live ISO" approach for IPI metal meant we wouldn't have this "use openstack on bare metal" stuff in the future.

pierreprinetti · 2022-01-11T14:50:20Z

We want to offer OpenShift-on-OpenStack users the ability to use managed baremetal nodes within OpenStack. Beyond the value of leveraging tools that the users might already know, I believe one further added value is to support a mixed fleet of virtual and physical servers within a single OCP cluster. Does that make sense?

mandre

Thanks @pierreprinetti for the patch.
/lgtm

what is the role of Ironic in OCP going forward? I thought the move to use the CoreOS "Live ISO" approach for IPI metal meant we wouldn't have this "use openstack on bare metal" stuff in the future.

This is about allowing OpenShift on OpenStack deployments to leverage ironic and provision BM worker nodes, it's unrelated to IPI metal.

openshift-bot · 2022-01-11T15:33:50Z

/retest-required

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

openshift-bot · 2022-01-11T16:34:05Z

/retest-required

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

mdbooth · 2022-01-11T16:45:35Z

This intersects a bit with a whole huge recent saga around hostnames; see https://bugzilla.redhat.com/show_bug.cgi?id=2008521#c40 and coreos/afterburn#509

A lot of confusion and overlap between "ownership" of the hostname. While this isn't new, the presence of this file is perpetuating a difference between FCOS and RHCOS behavior on OpenStack. (I think this is rooted in the uncertain availability of the metadata service across OpenStack deployments)

Another related topic; AIUI the reason for the current design is that availability of the config drive is racy. I don't know if that applies in Ironic or not.

The specific reason for the creation of a persistent hostname was a different in kubelet's behaviour when using legacy cloud provider vs external cloud provider. In summary: when using a legacy cloud provider kubelet gets its node name from the cloud provider; when using an external cloud provider kubelet gets its node name from hostname unless you pass it on the command line.

It turns out that in practise hostname is not always identical to the value returned by the cloud provider. It's an upgrade issue if either:

Hostname changes on an already-provisioned host
Nodename changes for an already-provisioned kubelet

Consequently we decided to use afterburn to fetch hostname from the cloud provider and pass it to kubelet via the command line option, which means node name remains the same across a legacy->external CCM upgrade, and we don't have to change the hostname.

mdbooth · 2022-01-11T16:46:58Z

/lgtm

Config drive with fallback to metadata is ideal for getting hostname.

openshift-ci · 2022-01-11T16:47:33Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cgwalters, mandre, mdbooth, pierreprinetti

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:

~~templates/common/openstack/OWNERS~~ [cgwalters,mandre,mdbooth,pierreprinetti]

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

openshift-bot · 2022-01-11T16:59:22Z

/retest-required

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

openshift-bot · 2022-01-11T17:12:24Z

/retest-required

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

openshift-bot · 2022-01-11T17:51:22Z

/retest-required

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

openshift-bot · 2022-01-11T18:04:23Z

/retest-required

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

openshift-bot · 2022-01-11T18:53:23Z

/retest-required

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

openshift-bot · 2022-01-11T19:05:22Z

/retest-required

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

openshift-bot · 2022-01-11T19:41:25Z

/retest-required

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

openshift-bot · 2022-01-11T19:53:23Z

/retest-required

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

openshift-bot · 2022-01-11T20:41:27Z

/retest-required

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

openshift-ci · 2022-01-11T22:01:24Z

@pierreprinetti: 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/e2e-aws-single-node	`60854e7`	link	false	`/test e2e-aws-single-node`
ci/prow/e2e-aws-workers-rhel7	`60854e7`	link	false	`/test e2e-aws-workers-rhel7`
ci/prow/e2e-aws-disruptive	`60854e7`	link	false	`/test e2e-aws-disruptive`
ci/prow/e2e-aws-upgrade-single-node	`60854e7`	link	false	`/test e2e-aws-upgrade-single-node`
ci/prow/e2e-aws-workers-rhel8	`60854e7`	link	false	`/test e2e-aws-workers-rhel8`
ci/prow/e2e-vsphere-upgrade	`60854e7`	link	false	`/test e2e-vsphere-upgrade`
ci/prow/e2e-ovn-step-registry	`60854e7`	link	false	`/test e2e-ovn-step-registry`

Full PR test history. Your PR dashboard.

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 · 2022-01-11T22:05:25Z

/retest-required

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

openshift-bot · 2022-01-11T22:54:24Z

/retest-required

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

openshift-bot · 2022-01-11T23:06:31Z

/retest-required

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

openshift-bot · 2022-01-11T23:42:22Z

/retest-required

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

openshift-bot · 2022-01-12T00:18:28Z

/retest-required

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

openshift-bot · 2022-01-12T00:30:23Z

/retest-required

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

openshift-bot · 2022-01-12T01:06:22Z

/retest-required

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

openshift-bot · 2022-01-12T01:19:22Z

/retest-required

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

openshift-bot · 2022-01-12T01:55:22Z

/retest-required

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

openshift-bot · 2022-01-12T02:44:29Z

/retest-required

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

openshift-ci · 2022-01-12T03:08:26Z

@pierreprinetti: All pull requests linked via external trackers have merged:

openshift/machine-config-operator#2903

Bugzilla bug 2033953 has been moved to the MODIFIED state.

In response to this:

Bug 2033953: Afterburn to try config-drive before Nova metadata

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.

pierreprinetti · 2022-01-17T08:37:48Z

/cherry-pick release-4.9

openshift-cherrypick-robot · 2022-01-17T08:38:31Z

@pierreprinetti: new pull request created: #2915

In response to this:

/cherry-pick release-4.9

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.

openshift-ci bot added bugzilla/severity-urgent Referenced Bugzilla bug's severity is urgent for the branch this PR is targeting. bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Jan 10, 2022

openshift-ci bot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Jan 10, 2022

openshift-ci bot requested review from eurijon, iamemilio and stephenfin January 10, 2022 14:42

openshift-ci bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Jan 10, 2022

openshift-ci bot requested a review from mandre January 10, 2022 15:24

mandre reviewed Jan 11, 2022

View reviewed changes

openshift-ci bot assigned mandre Jan 11, 2022

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 11, 2022

openshift-ci bot assigned mdbooth Jan 11, 2022

openshift-merge-robot merged commit 23bd462 into openshift:master Jan 12, 2022

pierreprinetti deleted the afterburn_provider branch January 12, 2022 17:05

openshift-cherrypick-robot mentioned this pull request Jan 17, 2022

[release-4.9] Bug 2041364: Afterburn to try config-drive before Nova metadata #2915

Closed

Bug 2033953: Afterburn to try config-drive before Nova metadata #2903

Bug 2033953: Afterburn to try config-drive before Nova metadata #2903

Conversation

pierreprinetti commented Jan 10, 2022 • edited Loading

openshift-ci bot commented Jan 10, 2022

pierreprinetti commented Jan 10, 2022

openshift-ci bot commented Jan 10, 2022

openshift-ci bot commented Jan 10, 2022

pierreprinetti commented Jan 10, 2022

cgwalters commented Jan 10, 2022

pierreprinetti commented Jan 11, 2022

cgwalters commented Jan 11, 2022

pierreprinetti commented Jan 11, 2022

mandre left a comment

Choose a reason for hiding this comment

openshift-bot commented Jan 11, 2022

openshift-bot commented Jan 11, 2022

mdbooth commented Jan 11, 2022

mdbooth commented Jan 11, 2022

openshift-ci bot commented Jan 11, 2022

openshift-bot commented Jan 11, 2022

openshift-bot commented Jan 11, 2022

openshift-bot commented Jan 11, 2022

openshift-bot commented Jan 11, 2022

openshift-bot commented Jan 11, 2022

openshift-bot commented Jan 11, 2022

openshift-bot commented Jan 11, 2022

openshift-bot commented Jan 11, 2022

openshift-bot commented Jan 11, 2022

openshift-ci bot commented Jan 11, 2022 • edited Loading

openshift-bot commented Jan 11, 2022

openshift-bot commented Jan 11, 2022

openshift-bot commented Jan 11, 2022

openshift-bot commented Jan 11, 2022

openshift-bot commented Jan 12, 2022

openshift-bot commented Jan 12, 2022

openshift-bot commented Jan 12, 2022

openshift-bot commented Jan 12, 2022

openshift-bot commented Jan 12, 2022

openshift-bot commented Jan 12, 2022

openshift-ci bot commented Jan 12, 2022

pierreprinetti commented Jan 17, 2022

openshift-cherrypick-robot commented Jan 17, 2022

pierreprinetti commented Jan 10, 2022 •

edited

Loading

openshift-ci bot commented Jan 11, 2022 •

edited

Loading