Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 84 additions & 2 deletions pkg/asset/agent/agentconfig/agenthosts.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/pkg/errors"
"github.com/sirupsen/logrus"
goyaml "gopkg.in/yaml.v2"
"k8s.io/apimachinery/pkg/util/validation/field"
"sigs.k8s.io/yaml"

Expand All @@ -16,6 +17,7 @@ import (
agentAsset "github.com/openshift/installer/pkg/asset/agent"
"github.com/openshift/installer/pkg/asset/agent/joiner"
"github.com/openshift/installer/pkg/asset/agent/workflow"
"github.com/openshift/installer/pkg/types"
"github.com/openshift/installer/pkg/types/agent"
"github.com/openshift/installer/pkg/types/baremetal/validation"
"github.com/openshift/installer/pkg/validate"
Expand All @@ -41,8 +43,9 @@ type nmStateInterface struct {
// AgentHosts generates the hosts information from the AgentConfig and
// OptionalInstallConfig assets.
type AgentHosts struct {
Hosts []agent.Host
rendezvousIP string
Hosts []agent.Host
FencingCredentialsHost []types.Credential
rendezvousIP string
}

// Name returns a human friendly name.
Expand Down Expand Up @@ -93,6 +96,9 @@ func (a *AgentHosts) Generate(_ context.Context, dependencies asset.Parents) err
}
}

// store per host fencing-credentials only when MAC address is used
a.populateFencingCredentialHosts(installConfig)

case workflow.AgentWorkflowTypeAddNodes:
a.Hosts = append(a.Hosts, addNodesConfig.Config.Hosts...)
for i, host := range a.Hosts {
Expand Down Expand Up @@ -367,5 +373,81 @@ func (a *AgentHosts) HostConfigFiles() (HostConfigFileMap, error) {
files[filepath.Join(name, "role")] = []byte(host.Role)
}
}

credsByDir := map[string][]*types.Credential{}
var dirOrder []string

for i := range a.FencingCredentialsHost {
cred := &a.FencingCredentialsHost[i]
dirName := findHostDirForMAC(files, cred.MACAddress)
if dirName == "" {
dirName = nextAvailableHostDir(files, len(a.Hosts))
files[filepath.Join(dirName, "mac_addresses")] = []byte(strings.ToLower(cred.MACAddress) + "\n")
}
if _, ok := credsByDir[dirName]; !ok {
dirOrder = append(dirOrder, dirName)
}
credsByDir[dirName] = append(credsByDir[dirName], cred)
}

for _, dir := range dirOrder {
cfg := &FencingCredentialsConfig{Credentials: credsByDir[dir]}
data, err := goyaml.Marshal(cfg)
if err != nil {
return nil, err
}
files[filepath.Join(dir, "fencing-credentials.yaml")] = data
}

return files, nil
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

func (a *AgentHosts) populateFencingCredentialHosts(installConfig *agentAsset.OptionalInstallConfig) {
if installConfig.Config == nil || installConfig.Config.ControlPlane == nil ||
installConfig.Config.ControlPlane.Fencing == nil {
return
}

for _, cred := range installConfig.Config.ControlPlane.Fencing.Credentials {
if cred.HostName == "" && cred.MACAddress != "" {
a.FencingCredentialsHost = append(a.FencingCredentialsHost, *cred)
}
}
}

func findHostDirForMAC(files HostConfigFileMap, macAddress string) string {
normalizedMAC := strings.ToLower(macAddress)
for key, content := range files {
if !strings.HasSuffix(key, "/mac_addresses") {
continue
}
dirName := strings.TrimSuffix(key, "/mac_addresses")
for _, mac := range strings.Split(strings.TrimSpace(string(content)), "\n") {
if strings.TrimSpace(mac) == normalizedMAC {
return dirName
}
}
}
return ""
}

func nextAvailableHostDir(files HostConfigFileMap, hostCount int) string {
existing := map[string]bool{}
for key := range files {
parts := strings.SplitN(key, "/", 2)
if len(parts) > 0 {
existing[parts[0]] = true
}
}
// Reserve host-<i> for every host slot, even those that emitted no
// files, so a MAC-only fencing credential never reuses that name.
for i := 0; i < hostCount; i++ {
existing[fmt.Sprintf("host-%d", i)] = true
}
for i := 0; ; i++ {
name := fmt.Sprintf("host-%d", i)
if !existing[name] {
return name
}
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
210 changes: 210 additions & 0 deletions pkg/asset/agent/agentconfig/agenthosts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -955,6 +955,216 @@ func (ib *InterfacetBuilder) build() *aiv1beta1.Interface {
return &ib.Interface
}

func TestAgentHosts_FencingCredentialsHost(t *testing.T) {
cases := []struct {
name string
dependencies []asset.Asset
expectedCredentials []types.Credential
}{
{
name: "mac-keyed credentials are collected",
dependencies: []asset.Asset{
&workflow.AgentWorkflow{Workflow: workflow.AgentWorkflowTypeInstall},
&joiner.AddNodesConfig{},
getInstallConfigWithFencing([]*types.Credential{
{MACAddress: "28:d2:44:d2:b2:1a", Username: "admin", Password: "pass", Address: "redfish+https://10.0.0.1/redfish/v1/Systems/1"},
}),
getAgentConfigMultiHost("worker"),
},
expectedCredentials: []types.Credential{
{MACAddress: "28:d2:44:d2:b2:1a", Username: "admin", Password: "pass", Address: "redfish+https://10.0.0.1/redfish/v1/Systems/1"},
},
},
{
name: "hostname-keyed credentials are excluded",
dependencies: []asset.Asset{
&workflow.AgentWorkflow{Workflow: workflow.AgentWorkflowTypeInstall},
&joiner.AddNodesConfig{},
getInstallConfigWithFencing([]*types.Credential{
{HostName: "master-0", Username: "admin", Password: "pass", Address: "redfish+https://10.0.0.1/redfish/v1/Systems/1"},
}),
getAgentConfigMultiHost("worker"),
},
expectedCredentials: nil,
},
{
name: "mixed credentials only collect mac-keyed",
dependencies: []asset.Asset{
&workflow.AgentWorkflow{Workflow: workflow.AgentWorkflowTypeInstall},
&joiner.AddNodesConfig{},
getInstallConfigWithFencing([]*types.Credential{
{HostName: "master-0", Username: "admin", Password: "pass", Address: "redfish+https://10.0.0.1/redfish/v1/Systems/1"},
{MACAddress: "28:d2:44:d2:b2:1b", Username: "admin2", Password: "pass2", Address: "redfish+https://10.0.0.2/redfish/v1/Systems/1"},
}),
getAgentConfigMultiHost("worker"),
},
expectedCredentials: []types.Credential{
{MACAddress: "28:d2:44:d2:b2:1b", Username: "admin2", Password: "pass2", Address: "redfish+https://10.0.0.2/redfish/v1/Systems/1"},
},
},
{
name: "credential with both hostname and mac is treated as hostname-keyed",
dependencies: []asset.Asset{
&workflow.AgentWorkflow{Workflow: workflow.AgentWorkflowTypeInstall},
&joiner.AddNodesConfig{},
getInstallConfigWithFencing([]*types.Credential{
{HostName: "master-0", MACAddress: "28:d2:44:d2:b2:1a", Username: "admin", Password: "pass", Address: "redfish+https://10.0.0.1/redfish/v1/Systems/1"},
}),
getAgentConfigMultiHost("worker"),
},
expectedCredentials: nil,
},
{
name: "no fencing config produces nil",
dependencies: []asset.Asset{
&workflow.AgentWorkflow{Workflow: workflow.AgentWorkflowTypeInstall},
&joiner.AddNodesConfig{},
getInstallConfigSingleHost(),
getAgentConfigSingleHost(),
},
expectedCredentials: nil,
},
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
parents := asset.Parents{}
parents.Add(tc.dependencies...)

ah := &AgentHosts{}
err := ah.Generate(context.Background(), parents)
assert.NoError(t, err)
assert.Equal(t, tc.expectedCredentials, ah.FencingCredentialsHost)
})
}
}

func TestHostConfigFiles_FencingCredentials(t *testing.T) {
cases := []struct {
name string
hosts []agent.Host
credentials []types.Credential
expectedFiles map[string]string
}{
{
name: "mac matches existing host directory",
hosts: []agent.Host{
{
Hostname: "master-0",
Interfaces: []*aiv1beta1.Interface{{MacAddress: "aa:bb:cc:dd:ee:01"}},
},
},
credentials: []types.Credential{
{MACAddress: "AA:BB:CC:DD:EE:01", Username: "admin", Password: "pass", Address: "redfish+https://10.0.0.1/redfish/v1/Systems/1"},
},
expectedFiles: map[string]string{
"master-0/fencing-credentials.yaml": "username: admin",
},
},
{
name: "mac with no matching host creates new directory",
hosts: []agent.Host{
{
Hostname: "master-0",
Interfaces: []*aiv1beta1.Interface{{MacAddress: "aa:bb:cc:dd:ee:01"}},
},
},
credentials: []types.Credential{
{MACAddress: "FF:FF:FF:FF:FF:FF", Username: "admin", Password: "pass", Address: "redfish+https://10.0.0.1/redfish/v1/Systems/1"},
},
expectedFiles: map[string]string{
"host-1/fencing-credentials.yaml": "username: admin",
"host-1/mac_addresses": "ff:ff:ff:ff:ff:ff",
},
},
{
name: "case-insensitive mac matching",
hosts: []agent.Host{
{
Hostname: "master-0",
Interfaces: []*aiv1beta1.Interface{{MacAddress: "aa:bb:cc:dd:ee:01"}},
},
},
credentials: []types.Credential{
{MACAddress: "AA:BB:CC:DD:EE:01", Username: "admin", Password: "pass", Address: "redfish+https://10.0.0.1/redfish/v1/Systems/1"},
},
expectedFiles: map[string]string{
"master-0/fencing-credentials.yaml": "username: admin",
},
},
{
name: "new dir avoids collision with existing host-0",
hosts: []agent.Host{
{
Interfaces: []*aiv1beta1.Interface{{MacAddress: "aa:bb:cc:dd:ee:01"}},
},
},
credentials: []types.Credential{
{MACAddress: "FF:FF:FF:FF:FF:FF", Username: "admin", Password: "pass", Address: "redfish+https://10.0.0.1/redfish/v1/Systems/1"},
},
expectedFiles: map[string]string{
"host-1/fencing-credentials.yaml": "username: admin",
"host-1/mac_addresses": "ff:ff:ff:ff:ff:ff",
},
},
{
name: "new dir avoids collision with empty host that has no files",
hosts: []agent.Host{
{},
},
credentials: []types.Credential{
{MACAddress: "FF:FF:FF:FF:FF:FF", Username: "admin", Password: "pass", Address: "redfish+https://10.0.0.1/redfish/v1/Systems/1"},
},
expectedFiles: map[string]string{
"host-1/fencing-credentials.yaml": "username: admin",
"host-1/mac_addresses": "ff:ff:ff:ff:ff:ff",
},
},
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
ah := &AgentHosts{
Hosts: tc.hosts,
FencingCredentialsHost: tc.credentials,
}

files, err := ah.HostConfigFiles()
assert.NoError(t, err)

for path, expectedContent := range tc.expectedFiles {
data, ok := files[path]
assert.True(t, ok, "expected file at %s", path)
assert.Contains(t, string(data), expectedContent)
}
})
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

func getInstallConfigWithFencing(credentials []*types.Credential) *agentAsset.OptionalInstallConfig {
ic := getNoHostsInstallConfig()
ic.Config.ControlPlane.Fencing = &types.Fencing{
Credentials: credentials,
}
return ic
}

func getAgentConfigNoHostname() *AgentConfig {
a := getNoHostsAgentConfig()
a.Config.Hosts = []agent.Host{
{
Role: "master",
Interfaces: []*aiv1beta1.Interface{
{
Name: "eth0",
MacAddress: "28:d2:44:d2:b2:1a",
},
},
},
}
return a
}

func getInstallConfigWithMismatchedNetworkConfig() *agentAsset.OptionalInstallConfig {
a := getInstallConfigSingleHost()
a.Config.Platform.BareMetal.Hosts[0].NetworkConfig = &apiextv1.JSON{
Expand Down
2 changes: 2 additions & 0 deletions pkg/asset/agent/agentconfig/fencingcredentials_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,8 @@ func TestFencingCredentials_YAMLFormat(t *testing.T) {
assert.NotContains(t, yamlContent, "macaddress:")
})

// This subtest verifies YAML format only. Placement into per-host directories
// for MAC-keyed credentials is handled by addFencingCredentials() in ignition.go.
t.Run("with macaddress", func(t *testing.T) {
parents := asset.Parents{}
parents.Add(
Expand Down
Loading