OPNET-592: Install-config interface for NMState configuration #1795
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@cybertron: This pull request references OPNET-592 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the epic to target the "4.20.0" version, but no target version was set. In response to this: 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 openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@cybertron: The following test failed, say
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-sigs/prow repository. I understand the commands that are listed here. |
| - 172.30.0.0/16 | ||
| # New part begins here | ||
| hostConfig: | ||
| - name: master-0 |
There was a problem hiding this comment.
What is the role of the name in this interface? Is it used to make a decision which configuration is applied to which node, or is it purely about labelling?
There are 2 cases where it may or may not matter
-
Bare metal clusters where you may not be sure if your network infra dispatches hostnames correctly (see wrong DHCP, missing PTRs and so on). If we match config for the NIC by its MAC address then it's cool because we don't really need a name for the host. At the same time, if you manage a cluster of metal nodes, you should have an inventory of MAC addresses, otherwise you have other, bigger, problems.
-
Non-metal platforms with machines dynamically created. Given the interface is now platform-agnostic, it is possible for someone to use it on e.g. AWS-deployed cluster. That means, at the moment of generating install-config you do not know names of the hosts; you also do not know MAC addresses of the NICs you are going to get on those nodes. Maybe this is a non-goal at this moment, but imagine a following scenario
As an operator deploying on AWS/Azure nodes with 2 NICs, I want the 2nd NIC of every node to be configured with a static IPv4 address in the form of
172.16.0.x/24 for x in 1-10.
We may or may not allow this, but interface in its current form looks like a perfect candidate to fulfil such a story.
There was a problem hiding this comment.
It's a mapping of configs to nodes. Currently the underlying feature only supports hostnames, but we could definitely look at supporting MACs too. I don't think the design proposed here would be incompatible with that - you'd just specify a MAC address instead of the hostname to identify the node.
For the purposes of platforms like cloud where you don't know either the hostname or MAC ahead of time, there are a couple of mitigating factors:
- Those are the least likely platforms to want this functionality because they tend to have less complex networking (I know that's not universally true, but in general it is).
- We don't support per-host configuration on those platforms today, and they can still use the cluster.yml configuration to provide a cluster-wide configuration if they so desire.
There was a problem hiding this comment.
MachineConfigs don't support selecting individual nodes. Is this doing some sort of runtime selection like ABI/Assisted?
ABI/Assisted and NNCP both use a list of interface MACs to select nodes, it would be nice to keep that consistent.
There was a problem hiding this comment.
We're currently using hostname, but adding MAC-based mapping makes a lot of sense too. I don't think anything about this design precludes having different identifiers in the future. Anything unique to a specific node should work.
|
/lgtm |
| complementary. The baremetal feature can be used to configure, for | ||
| example, a static IP on the primary interface to allow Ignition to be | ||
| pulled, then this feature can be used to do full configuration on the | ||
| node. |
There was a problem hiding this comment.
We also need to consider agent installer, which if IIUC exposes complete nmstate configuration: https://github.com/openshift/assisted-service/blob/master/api/v1beta1/nmstate_config_types.go
Looping in @andfasano & @zaneb
There was a problem hiding this comment.
baremetal platform and ABI usage is similar. They're both used for pre-provisioning configuration (i.e. before the host becomes a node) but then also copied into the node.
| complementary. The baremetal feature can be used to configure, for | ||
| example, a static IP on the primary interface to allow Ignition to be | ||
| pulled, then this feature can be used to do full configuration on the | ||
| node. |
There was a problem hiding this comment.
baremetal platform and ABI usage is similar. They're both used for pre-provisioning configuration (i.e. before the host becomes a node) but then also copied into the node.
|
|
||
| Add a new field to the networking section of install-config that allows | ||
| users to provide their NMState config for cluster nodes. This will look | ||
| something like the networkConfig field for the baremetal platform, but |
There was a problem hiding this comment.
If only one is specified, will the other default to that? In both directions or only one direction? If the latter, which?
There was a problem hiding this comment.
No, in this case it's not possible for the same config to be used for both steps. At Ignition pull time we can't have br-ex because OVS isn't running yet, so that initial config has to be different from the final one.
|
|
||
| A user will write an appropriate NMState config for their network environment | ||
| and include that in their install-config. The NMState YAML they provide will | ||
| then be base64-encoded and placed in a machine-config manifest that will be |
There was a problem hiding this comment.
Is it time to reopen the NNCP debate?
There was a problem hiding this comment.
That would streamline a lot of our day one configuration. In this case we already have the feature and it's being used so putting a nicer interface on it is probably necessary whether we get NNCP integration for day one or not, but that's definitely a conversation we need to revisit.
| - 172.30.0.0/16 | ||
| # New part begins here | ||
| hostConfig: | ||
| - name: master-0 |
There was a problem hiding this comment.
MachineConfigs don't support selecting individual nodes. Is this doing some sort of runtime selection like ABI/Assisted?
ABI/Assisted and NNCP both use a list of interface MACs to select nodes, it would be nice to keep that consistent.
| networkConfig: | ||
| interfaces: | ||
| - type: ethernet | ||
| name: eth0 |
There was a problem hiding this comment.
ABI/Assisted both have a feature where the interface name gets substituted at runtime based on the matching the MAC to a real interface. I guess the thinking here is that this is no longer required because NMState supports it directly? (And hopefully one day will allow VLANs on top?)
There was a problem hiding this comment.
Yes, I don't want to implement something OpenShift-specific when NMState already has most of what we need, and the remaining bits are doable on their side.
| ``` | ||
|
|
||
| The content of the networkConfig fields will be used as input to the | ||
| configure-ovs-alternative feature. |
There was a problem hiding this comment.
Are we expecting the OVS bridge configuration itself to be modifiable through this mechanism?
There was a problem hiding this comment.
Yes and no. Right now this is a one-time-only configuration and can't be used on day two to modify things (the NMState config needs to be copied to Kubernetes NMState for day two). However, with Kubernetes-NMState you are allowed to make limited modifications to br-ex. We're also investigating reusing this mechanism to make more disruptive changes that would require a reboot: https://issues.redhat.com/browse/OPNET-594
|
|
||
| All of this configuration can already be achieved by writing nmconnection | ||
| files to the nodes anyway. This is actually a slight improvement in that | ||
| NMState will validate syntax before applying changes and will roll back |
There was a problem hiding this comment.
We can't validate this well in the installer, so the reality is that bad syntax will result in the config being more or less silently ignored.
There was a problem hiding this comment.
Yeah, we can't validate it early, but it will cause a failure at deploy-time for the node, and hopefully leave you with a functional network configuration (albeit without br-ex) so at least you can get on the node to debug easily.
| new virtualization customers will have limited experience with advanced | ||
| host network configuration and want to make the experience as smooth as | ||
| possible. While the existing machine-config interface is functional, it | ||
| requires manual creation of base64-encoded content and machine-config |
There was a problem hiding this comment.
Butane allows defining data inline, and can be processed into a MachineConfig (many examples of this in the OpenShift docs), so it's not quite this dire.
| name: eth0 | ||
| ... | ||
| - name: master-1 | ||
| networkConfig: |
There was a problem hiding this comment.
Assisted (but not ABI) already has plans for NMPolicy support. Would we consider that valid here?
There was a problem hiding this comment.
I believe so. Anything you can apply with the nmstate service on the node (the built-in one from RHEL, not the nmstate-configuration service we add from OCP to map these configs). In fact, I'm pretty sure at least one existing user of this feature is using policy-based configurations because they have a separate way of tagging interfaces for their intended use.
No description provided.