On a netplan-rendered node the reconfigure path takes the interface down
unconditionally, while the `netplan apply` that brings it back is gated on
reboot_nic_bool. In the diskful provision postscripts stage reboot_nic_bool is
0, so configeth downs the NIC and nothing brings it back until the node
reboots -- over the very interface the postscripts are talking on.
The other two arms pair their halves: the ifupdown arm answers `ifdown` with an
unconditional `ifup`, and the redhat arm gates both the down and the up. Only
the netplan arm takes one half.
Nothing in the suite covers it. This drives the two blocks configeth actually
executes -- the down-selection inside the modify branch and the restart block
underneath it -- with `ip`, `ifdown`, `ifup` and `netplan` shadowed by shell
functions, which bash resolves ahead of $PATH, so every call is recorded rather
than run and the host's network is never touched.
Six of the eight assertions pass already and are there to pin the arms that are
correct: redhat gating both halves, ifupdown pairing its own, and the
reboot_nic_bool=1 path doing the down before the apply. The two that fail are
the bug.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
The test shipped with the netplan support drove only the private
write_netplan_* writers, so it could not see the branch selection that is
the actual fix: deleting the netplan arm from configipv4 outright left all
fourteen assertions green. Its one real-parser gate was vacuous as well --
`netplan generate --root-dir` reads <root>/etc/netplan, and the harness
wrote the drop-ins flat into the scratch directory, so netplan parsed zero
files and exited 0 for any content at all, including a key it rejects.
Drive configipv4, configipv6 and delete_nic_config_files instead, and let
the generate gate see the tree it is meant to validate. netplan, ifup and
wait_for_ifstate are shadowed with shell functions, which bash resolves
ahead of $PATH, so `netplan apply` is recorded rather than run and the
suite cannot touch the host's network however it is invoked.
Two assertions had pinned the behaviour that was wrong: that a VLAN file
declares no ethernets: section -- which is exactly why netplan could not
resolve its link: -- and that the default route reads "to: default", an
alias netplan only understands from 0.103.
The harness now also uses the sentinel configeth really sets rather than
one of its own, so an unset MTU is exercised through the comparison the
script performs instead of past it.
Each assertion was checked by reverting the behaviour it describes: all
nine reversions fail, including the one the previous test was blind to.
On a runner with netplan present the generate gate fails with
"unknown key 'MTU'" when nicextraparams are passed through unmapped.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
The netplan writer added for issue #7454 emits configuration that netplan
refuses to load, and netplan parses everything under /etc/netplan as one
document -- so a single rejected key takes down the whole node's network,
not just the interface being configured. It also left the install NIC, the
path every provisioned node actually takes, still writing to interfaces.d.
Verified against netplan 1.1.2; every case below was reproduced as a
`netplan generate` failure before the change and passes after it.
- A VLAN's parent was never declared. netplan resolves `link:` at parse
time and does not look ahead, and 90-xcat-<parent>.<vid>.yaml sorts
before 90-xcat-<parent>.yaml anyway; a parent carrying no address of
its own got no file at all. Declare it as an empty netdef in the same
file, which resolves the reference without overriding the parent's
own keys.
- nicextraparams were pasted in as netplan keys. They are ifcfg names --
the documented example is "MTU=1456 ONBOOT=no" -- and an unknown key
fails the file. Map the ones with a netplan equivalent, drop the rest
with a warning, and quote values so a ':' cannot invent a key.
- `to: default` needs netplan 0.103. Ubuntu 18.04 never ships past 0.99
and a stock 20.04 ships 0.99, so the IPv6 gateway rejected the file on
the very releases the support targets. Emit the explicit CIDR instead.
- Nothing set dhcp4/dhcp6. netplan merges same-id netdefs key by key
rather than replacing them, so cloud-init's dhcp4:true survived and the
node ran a DHCP lease alongside the static address xCAT had assigned.
- State was append-only: an address removed from the nics table was never
dropped, so a re-addressed NIC kept its old IP forever. Reset the
drop-in for the first address of a NIC, as the ifupdown branch's
truncating '>' does.
Beyond what netplan rejects:
- configeth -s wrote /etc/network/interfaces.d for the install NIC, which
is exactly the file the change exists to stop using. Write the drop-in.
- The -s bring-up tested netplan in an elif after NetworkManager, so on an
NM-rendered node it was dead code falling through to nmcli with an unset
connection name. Test netplan first, as the main path already did.
- configipv6 passed $str_nic_mtu, which only configipv4 ever assigns, and
dropped nicextraparams entirely.
- The -r path removed the drop-in without applying it, so the address
stayed on the link, and delete_nic_config_files skipped the
xcat_history_important cleanup its three sibling branches all do.
- netplan_active was true whenever netplan.io was installed, which is
every Ubuntu; require ifupdown to be absent or stopped.
- A nicextraparams name went unescaped into a sed address, so a '/' in one
errored out and a metacharacter could delete unrelated recorded state.
- `netplan apply` returns 0 without having brought the link up, and the
ifdown sites the -r and reconfigure paths use do not exist on a netplan
node.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Write /etc/netplan/90-xcat-<nic>.yaml and `netplan apply` on Debian/Ubuntu nodes whose
network is rendered by netplan, instead of /etc/network/interfaces.d/* which ifupdown
is no longer present to read. The legacy branch is kept for nodes that still run
ifupdown, selected on whether netplan is actually installed. Fixes#7454.
The drop-in is regenerated in full on every change rather than edited in place. Its
inputs are recorded in "# xcat-state:" comment lines in the same file, which netplan
ignores, so the file stays the single source of truth and re-rendering is idempotent.
That is what makes the three properties below hold; an in-place sed editor cannot.
A NIC named <parent>.<vid> is emitted under vlans: with id and link, so netplan
recreates the VLAN after a reboot -- the ifupdown branch has always written
vlan-raw-device for these, and declaring them as plain ethernets would lose that. A
dotted name whose suffix is not numeric stays an ethernet.
Addresses keep the order they were added, and a route is deduplicated on the whole
(to, via) pair so a second route sharing a gateway is still written.
nicextraparams are carried across into the interface stanza, matching what the
ifupdown branch does with them.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Ubuntu 18.04 and later render the network with netplan. ifupdown is not installed and
/etc/network/interfaces.d/* is ignored entirely, so configeth's Debian branch -- which
writes exactly there -- configures nothing at all on a modern Ubuntu node. It must
write /etc/netplan/*.yaml and apply it instead. Issue #7454.
Three properties a netplan writer has to hold, all of which an in-place YAML editor
gets wrong. A VLAN interface (<parent>.<vid>) belongs under vlans: with id and link,
or netplan never recreates it after a reboot. Multiple addresses on one NIC must keep
the order they were added. And routes must be idempotent on the whole (to, via) pair,
not on either field alone, or a second route sharing a gateway is swallowed.
nicextraparams must survive as well: the ifupdown branch writes them into the
interface stanza, so a netplan branch that drops them silently discards configuration
the user asked for.
Drive the real writers -- extracted from configeth and run against a temp NETPLAN_DIR
-- and hand the result to `netplan generate` where netplan is installed.
Signed-off-by: Daniel Hilst <392820+dhilst@users.noreply.github.com>
Resolve optional Mellanox network drivers against the target image's selected kernel. Keep real mlx_en modules, fall back to mlx4_en, include available mlx5_core, and omit missing defaults without changing explicitly requested netdrivers.
Extract secret_in_request, secret_in_response and the secret sets from
xcatd.pm, because the classifier and the finalizer consult them. Cover
the authentication key, the privacy key and the snmpc site value reads
as sensitive, the token, prodkey and site table dumps as sensitive, the
nodels expansion of a secret table as sensitive, the lsvm directory
listing as sensitive, and the implicit lsdef attribute listing as
redacted. Keep a plain site value, a benign table,
a benign object listing and a group named like a table as not
sensitive. Assert that the daemon redacts each request segment alone.
Build a medium for each CentOS Linux 8 release from the package names that
the release gives, and make sure that copycds finds the minor version.
Pin the behaviour when the medium gives no release package, and make sure
that a package with the same centos-release prefix, or a package for a
different major version, does not give a version. Pin that a medium that
names two minor versions keeps the unversioned name. Pin that CentOS Stream,
Rocky Linux, AlmaLinux, Red Hat Enterprise Linux and Oracle Linux keep
their own branches.
Every CentOS Linux 8 medium gives the same description, "CentOS Linux 8".
The description gives no minor version. The .treeinfo file on the medium
gives no minor version. Thus copycds gave the name centos8 to all of these
media. Two CentOS Linux 8 media then wrote into the same /install/centos8
directory and made osimage definitions with the same names.
The discinfo identifier table gives the minor version, but it contains
only 8.1 and 8.5. The identifiers of an expanded tree are also different
from the identifiers of the DVD. The release package in BaseOS/Packages is
the only other record of the minor version on the medium.
Read the version from the name of that package. CentOS changed the name of
the package to centos-linux-release in 8.3, thus accept the two names.
Accept only a major.minor version, because other packages start with the
same centos-release prefix. Accept only the major version that the
description gives, because a package for a different major version does not
describe this medium. Accept the minor version only when the medium names
one, because a medium that names more than one does not pin a minor version.
Keep the version from the description in the other conditions. A medium that
names no minor version keeps the name centos8, which is the behaviour before
this change and the correct name for media that do not pin a minor version.
CentOS Stream, Rocky Linux, AlmaLinux and Red Hat Enterprise Linux keep
their own branches. CentOS Linux 7 gives a different description and has
no BaseOS directory, thus it does not use this path.
Recovered from the lenovobuild branch. Reimplemented against master: the
original reads only centos-release, which CentOS renamed in 8.3, and its
expression is not anchored.
The commands.log response classifier used a "passw" text match on the
request arguments. A secret whose name has no such text passed the
check, so a read of an authentication key, a privacy key or the snmpc
site value logged its bare value in the response. A command that
expands an argument also passed the check: nodels with a table name
returns every column of the table, and lsdef returns attributes that
the request never names. The daemon also ran redact_password over the
whole connection log on each request, so the redactor split at the
first request of the connection and the change signal swept the text of
earlier requests and responses.
Add secret_in_request. The routine reports a request that names a
secret attribute, selects a secret site key, or dumps a table that owns
a secret column through tabdump or nodels, from the same secret set
that the argument redaction uses. The response classifier calls it, so
the response of such a request logs as redacted.
Add secret_in_response. The routine reports response text that holds
"passw" or a secret attribute name in assignment or column form. The
response finalizer calls it in place of the bare text match, so an
expanded listing that carries an authentication key or a product key
logs as redacted even when the request never names it. The lsvm
response is the directory entry, whose passwords are positional, so
the classifier marks the command itself.
Build each request segment alone, redact the segment, and then append
it to the connection log. The redactor now always sees the current
command, and the change signal covers only the current request.
With PDU2-MIB loaded, SNMP.pm returns an enumerated INTEGER as its label
rather than its number: a PX4 answers inletSensorUnits with "amp" rather
than "2". That failed the numeric test, fell back to none(-1) and
dropped the unit suffix from every rvitals reading.
Add pdu2_enum, which accepts the number, the label, or the label(value)
form Net-SNMP produces with quick printing off, and use it for the
sensor units and for outletSwitchingState.
Also stop requiring the switching probe to read on or off. An outlet can
be in any SensorStateEnumeration state, and pdu2_get has already ruled
out an absent instance, so any answer proves the outlet is switchable.
Signed-off-by: Kilian Cavalotti <kilian@stanford.edu>
Record that a node with more than one BMC is configured one BMC at a
time and that a comma separated value gives one setting per BMC, in
both the man page source and the checked-in text.
Add a unit test for the entry decision of the blade preprocessor. The test
lifts the entry out of the plugin source and drives it, because the plugin
needs a management node to load in full.
The test gives the entry the request that a booting node sends, which names
no node, and shows that the entry hands it on and answers no error. It also
shows that a request for another command without a noderange is still
refused.
Extract per_bmc_argument and the set of settings that take a list from
ipmi.pm, and drive the real routine, because the plugin needs daemon
dependencies that the test host does not have.
Cover a single value serving every BMC, a list read in session order, a
list too short for the BMC being configured, empty entries, and the
defensive inputs. Assert that only ip, netmask and gateway read a list,
that the caller reports a short list rather than sending it, that the
session keeps the value of its own BMC for the follow-up, and that the
thermal profile keeps its own argument.
A node that boots sends a findme request to say that it is there. That
request names no node, because the node is what the request asks xCAT to
find. The preprocessor of the blade plugin asks for a noderange for every
command, so it answers "Missing Noderange" with an error code and gives back
no request. The daemon then has nothing to give to the handler, and the
findme handler of the plugin does not run.
The plugin therefore answers each discovery request with an error, and blade
discovery does not happen. The check that asks for a noderange is older than
the findme handler.
Hand a findme request to the handler, as the switch plugin does for the
commands that it does not preprocess. Every other command keeps the check.
The preprocessor also held a test that removed a node from a findme request
when the hardware type was not blade. A findme request now returns above that
test, and it named a noderange that a findme request never carries, so the
test could not run. Remove it.
Add a unit test for the routine that selects the blades. The test lifts the
routine out of the plugin source, because the plugin needs a management node
to load.
The test gives the routine the row shapes that the writers of the mp table
make: a management module and its blades from xCAT::PPCdb::add_systemX and
the shipped mp template, a chassis that gives its hardware type, and one row
for each hardware type that lsslp writes. It also shows that the handler
reads the chassis attribute and that it stops before the arp table when the
mp table holds no blades.
A node can carry more than one BMC, and rspconfig already opens a
session per BMC for rinv, reventlog, rvitals and rspconfig. A setting
such as ip= carried a single value, so every BMC of the node received
the same one. Two BMCs cannot share an address, so a node like that
could not be configured through rspconfig at all.
Read a comma separated value as one setting per BMC, in the order the
sessions are numbered. Only the ip, netmask and gateway settings read a
list, because a comma belongs to the value itself in a free form SNMP
community string. A value without a comma still reaches every BMC, so
the existing single BMC use is unchanged.
An entry that is missing or empty reports the mismatch instead of
reaching the address encoders, which reject an empty string. The
session then holds the value of its own BMC, because the follow-up
callbacks read the subcommand again to decide whether the address came
from DHCP.
Recovered from the lenovobuild branch.
The findme handler of the blade plugin makes an inventory request for each
node in the mp table. That table holds more than the blades of a chassis.
lsslp writes a row for a Power BMC, for an FSP, for a BPA, for an HMC and for
other hardware, and xCAT::PPCdb::add_systemX writes a row for a management
module. None of that hardware answers a blade inventory.
Keep a row that gives blade as its hardware type. Keep also a row that gives
no hardware type but names a different node as its mpa, when that other node
is a management module. The mp template in xCAT/templates/e1350 leaves the
hardware type of a blade empty, so a test of the hardware type alone loses
the blades of a chassis.
Return when the table holds no blades. The work below the filter reads the
arp table of the management node, and a site that has no chassis must not pay
for that on each discovery request.
Recovered from the lenovobuild branch, which tested the hardware type only.
Daniel Hilst
Vinícius Ferrão
Vinícius Ferrão
Kilian Cavalotti