Skip to content

Commit

Permalink
Remove lazy loading of hardware in tinkerbell validation (#1970)
Browse files Browse the repository at this point in the history
  • Loading branch information
@chrisdoherty4
chrisdoherty4 authored May 3, 2022
1 parent 23f438c commit 71fae6a
Show file tree
Hide file tree
Showing 5 changed files with 144 additions and 157 deletions.
156 changes: 83 additions & 73 deletions pkg/providers/tinkerbell/hardware/config.go → ...roviders/tinkerbell/hardware/catalogue.go
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package hardware

import (
"bufio"
"encoding/json"
"errors"
"fmt"
"io/ioutil"
"strings"
"io"
"os"

pbnjv1alpha1 "github.com/tinkerbell/cluster-api-provider-tinkerbell/pbnj/api/v1alpha1"
tinkv1alpha1 "github.com/tinkerbell/cluster-api-provider-tinkerbell/tink/api/v1alpha1"
Expand All @@ -14,6 +16,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
apimachineryvalidation "k8s.io/apimachinery/pkg/util/validation"
yamlutil "k8s.io/apimachinery/pkg/util/yaml"
"sigs.k8s.io/yaml"

"github.com/aws/eks-anywhere/pkg/api/v1alpha1"
Expand All @@ -25,69 +28,22 @@ import (

const Provisioning = "provisioning"

type HardwareConfig struct {
Hardwares []tinkv1alpha1.Hardware
Bmcs []pbnjv1alpha1.BMC
Secrets []corev1.Secret
type Catalogue struct {
Hardware []tinkv1alpha1.Hardware
BMCs []pbnjv1alpha1.BMC
Secrets []corev1.Secret
}

func (hc *HardwareConfig) ParseHardwareConfig(hardwareFileName string) error {
err := hc.setHardwareConfigFromFile(hardwareFileName)
if err != nil {
return fmt.Errorf("unable to parse hardware file %s: %v", hardwareFileName, err)
}
return nil
}

func (hc *HardwareConfig) setHardwareConfigFromFile(hardwareFileName string) error {
content, err := ioutil.ReadFile(hardwareFileName)
if err != nil {
return fmt.Errorf("unable to read file due to: %v", err)
}

for _, c := range strings.Split(string(content), v1alpha1.YamlSeparator) {
var resource unstructured.Unstructured
if err = yaml.Unmarshal([]byte(c), &resource); err != nil {
return fmt.Errorf("unable to parse %s\nyaml: %s\n %v", hardwareFileName, c, err)
}
switch resource.GetKind() {
case "Hardware":
var hardware tinkv1alpha1.Hardware
err = yaml.UnmarshalStrict([]byte(c), &hardware)
if err != nil {
return fmt.Errorf("unable to parse hardware CRD\n%s \n%v", c, err)
}
hc.Hardwares = append(hc.Hardwares, hardware)
case "BMC":
var bmc pbnjv1alpha1.BMC
err = yaml.UnmarshalStrict([]byte(c), &bmc)
if err != nil {
return fmt.Errorf("unable to parse bmc CRD\n%s \n%v", c, err)
}
hc.Bmcs = append(hc.Bmcs, bmc)
case "Secret":
var secret corev1.Secret
err = yaml.UnmarshalStrict([]byte(c), &secret)
if err != nil {
return fmt.Errorf("unable to parse k8s secret\n%s \n%v", c, err)
}
hc.Secrets = append(hc.Secrets, secret)
}
}

return nil
}

func (hc *HardwareConfig) ValidateHardware(skipPowerActions, force bool, tinkHardwareMap map[string]*tinkhardware.Hardware, tinkWorkflowMap map[string]*tinkworkflow.Workflow) error {
func (c *Catalogue) ValidateHardware(skipPowerActions, force bool, tinkHardwareMap map[string]*tinkhardware.Hardware, tinkWorkflowMap map[string]*tinkworkflow.Workflow) error {
bmcRefMap := map[string]*tinkv1alpha1.Hardware{}
if !skipPowerActions {
bmcRefMap = hc.initBmcRefMap()
bmcRefMap = c.initBmcRefMap()
}

// A database of observed hardware IDs so we can check for uniqueness.
hardwareIdsDb := make(map[string]struct{}, len(hc.Hardwares))
hardwareIdsDb := make(map[string]struct{}, len(c.Hardware))

for _, hw := range hc.Hardwares {
for _, hw := range c.Hardware {
if hw.Name == "" {
return fmt.Errorf("hardware name is required")
}
Expand Down Expand Up @@ -160,10 +116,10 @@ func (hc *HardwareConfig) ValidateHardware(skipPowerActions, force bool, tinkHar
return nil
}

func (hc *HardwareConfig) ValidateBMC() error {
secretRefMap := hc.initSecretRefMap()
bmcIpMap := make(map[string]struct{}, len(hc.Bmcs))
for _, bmc := range hc.Bmcs {
func (c *Catalogue) ValidateBMC() error {
secretRefMap := c.initSecretRefMap()
bmcIpMap := make(map[string]struct{}, len(c.BMCs))
for _, bmc := range c.BMCs {
if bmc.Name == "" {
return fmt.Errorf("bmc name is required")
}
Expand Down Expand Up @@ -198,8 +154,8 @@ func (hc *HardwareConfig) ValidateBMC() error {
return nil
}

func (hc *HardwareConfig) ValidateBmcSecretRefs() error {
for _, s := range hc.Secrets {
func (c *Catalogue) ValidateBmcSecretRefs() error {
for _, s := range c.Secrets {
if s.Name == "" {
return fmt.Errorf("secret name is required")
}
Expand Down Expand Up @@ -228,34 +184,34 @@ func (hc *HardwareConfig) ValidateBmcSecretRefs() error {
return nil
}

func (hc *HardwareConfig) initBmcRefMap() map[string]*tinkv1alpha1.Hardware {
bmcRefMap := make(map[string]*tinkv1alpha1.Hardware, len(hc.Bmcs))
for _, bmc := range hc.Bmcs {
func (c *Catalogue) initBmcRefMap() map[string]*tinkv1alpha1.Hardware {
bmcRefMap := make(map[string]*tinkv1alpha1.Hardware, len(c.BMCs))
for _, bmc := range c.BMCs {
bmcRefMap[bmc.Name] = nil
}

return bmcRefMap
}

func (hc *HardwareConfig) initSecretRefMap() map[string]struct{} {
secretRefMap := make(map[string]struct{}, len(hc.Secrets))
for _, s := range hc.Secrets {
func (c *Catalogue) initSecretRefMap() map[string]struct{} {
secretRefMap := make(map[string]struct{}, len(c.Secrets))
for _, s := range c.Secrets {
secretRefMap[s.Name] = struct{}{}
}

return secretRefMap
}

func (hc *HardwareConfig) HardwareSpecMarshallable() ([]byte, error) {
func (c *Catalogue) HardwareSpecMarshallable() ([]byte, error) {
var marshallables []v1alpha1.Marshallable

for _, hw := range hc.Hardwares {
for _, hw := range c.Hardware {
marshallables = append(marshallables, hardwareMarshallable(hw))
}
for _, bmc := range hc.Bmcs {
for _, bmc := range c.BMCs {
marshallables = append(marshallables, bmcMarshallable(bmc))
}
for _, secret := range hc.Secrets {
for _, secret := range c.Secrets {
marshallables = append(marshallables, secretsMarshallable(secret))
}

Expand Down Expand Up @@ -314,3 +270,57 @@ func secretsMarshallable(secret corev1.Secret) *corev1.Secret {

return config
}

// ParseYAMLCatalogueFromFile parses filename, a YAML document, using ParseYamlCatalogue.
func ParseYAMLCatalogueFromFile(config *Catalogue, filename string) error {
fh, err := os.Open(filename)
if err != nil {
return err
}

return ParseYAMLCatalogue(config, fh)
}

// ParseCatalogue parses a YAML document, r, that represents a set of Kubernetes manifests.
// Manifests parsed include CAPT Hardware, PBnJ BMCs and associated Core API Secret.
func ParseYAMLCatalogue(catalogue *Catalogue, r io.Reader) error {
document := yamlutil.NewYAMLReader(bufio.NewReader(r))
for {
manifest, err := document.Read()
if errors.Is(err, io.EOF) {
return nil
}
if err != nil {
return err
}

var resource unstructured.Unstructured
if err = yaml.Unmarshal(manifest, &resource); err != nil {
return err
}

switch resource.GetKind() {
case "Hardware":
var hardware tinkv1alpha1.Hardware
err = yaml.UnmarshalStrict(manifest, &hardware)
if err != nil {
return fmt.Errorf("unable to parse hardware manifest: %v", err)
}
catalogue.Hardware = append(catalogue.Hardware, hardware)
case "BMC":
var bmc pbnjv1alpha1.BMC
err = yaml.UnmarshalStrict(manifest, &bmc)
if err != nil {
return fmt.Errorf("unable to parse bmc manifest: %v", err)
}
catalogue.BMCs = append(catalogue.BMCs, bmc)
case "Secret":
var secret corev1.Secret
err = yaml.UnmarshalStrict(manifest, &secret)
if err != nil {
return fmt.Errorf("unable to parse secret manifest: %v", err)
}
catalogue.Secrets = append(catalogue.Secrets, secret)
}
}
}
51 changes: 27 additions & 24 deletions pkg/providers/tinkerbell/tinkerbell.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,14 @@ type tinkerbellProvider struct {
providerTinkClient ProviderTinkClient
pbnj ProviderPbnjClient
templateBuilder *TinkerbellTemplateBuilder
hardwareConfigFile string
validator *Validator
writer filewriter.FileWriter
keyGenerator SSHAuthKeyGenerator

hardwareManifestPath string
// catalogue is a cache initialized during SetupAndValidateCreateCluster() from hardwareManifestPath.
catalogue hardware.Catalogue

skipIpCheck bool
skipPowerActions bool
setupTinkerbell bool
Expand Down Expand Up @@ -128,7 +131,7 @@ func NewProvider(
providerTinkbellClient TinkerbellClients,
now types.NowFunc,
skipIpCheck bool,
hardwareConfigFile string,
hardwareManifestPath string,
skipPowerActions bool,
setupTinkerbell bool,
force bool,
Expand All @@ -144,7 +147,7 @@ func NewProvider(
&networkutils.DefaultNetClient{},
now,
skipIpCheck,
hardwareConfigFile,
hardwareManifestPath,
skipPowerActions,
setupTinkerbell,
force,
Expand All @@ -162,7 +165,7 @@ func NewProviderCustomDep(
netClient networkutils.NetClient,
now types.NowFunc,
skipIpCheck bool,
hardwareConfigFile string,
hardwareManifestPath string,
skipPowerActions bool,
setupTinkerbell bool,
force bool,
Expand Down Expand Up @@ -197,9 +200,9 @@ func NewProviderCustomDep(
etcdMachineSpec: etcdMachineSpec,
now: now,
},
hardwareConfigFile: hardwareConfigFile,
validator: NewValidator(providerTinkClient, netClient, hardware.HardwareConfig{}, pbnjClient),
writer: writer,
hardwareManifestPath: hardwareManifestPath,
validator: NewValidator(providerTinkClient, netClient, pbnjClient),
writer: writer,

// (chrisdoherty4) We're hard coding the dependency and monkey patching in testing because the provider
// isn't very testable right now and we already have tests in the `tinkerbell` package so can monkey patch
Expand Down Expand Up @@ -238,7 +241,7 @@ func (p *tinkerbellProvider) PreCAPIInstallOnBootstrap(ctx context.Context, clus

func (p *tinkerbellProvider) PostBootstrapSetup(ctx context.Context, clusterConfig *v1alpha1.Cluster, cluster *types.Cluster) error {
// TODO: figure out if we need something else here
hardwareSpec, err := p.validator.hardwareConfig.HardwareSpecMarshallable()
hardwareSpec, err := p.catalogue.HardwareSpecMarshallable()
if err != nil {
return fmt.Errorf("failed marshalling resources for hardware spec: %v", err)
}
Expand Down Expand Up @@ -366,7 +369,7 @@ func (p *tinkerbellProvider) configureSshKeys() error {
func (p *tinkerbellProvider) SetupAndValidateCreateCluster(ctx context.Context, clusterSpec *cluster.Spec) error {
logger.Info("Warning: The tinkerbell infrastructure provider is still in development and should not be used in production")

hardware, err := p.providerTinkClient.GetHardware(ctx)
tinkHardware, err := p.providerTinkClient.GetHardware(ctx)
if err != nil {
return fmt.Errorf("retrieving tinkerbell hardware: %v", err)
}
Expand All @@ -382,10 +385,14 @@ func (p *tinkerbellProvider) SetupAndValidateCreateCluster(ctx context.Context,
return err
}

// ValidateHardwareConfig performs a lazy load of hardware configuration. Given subsequent steps need the hardware
if err := hardware.ParseYAMLCatalogueFromFile(&p.catalogue, p.hardwareManifestPath); err != nil {
return err
}

// ValidateHardwareCatalogue performs a lazy load of hardware configuration. Given subsequent steps need the hardware
// read into memory it needs to be done first. It also needs connection to
// Tinkerbell steps to verify hardware availability on the stack
if err := p.validator.ValidateHardwareConfig(ctx, p.hardwareConfigFile, hardware, p.skipPowerActions, p.force); err != nil {
if err := p.validator.ValidateHardwareCatalogue(ctx, p.catalogue, tinkHardware, p.skipPowerActions, p.force); err != nil {
return err
}

Expand All @@ -400,11 +407,11 @@ func (p *tinkerbellProvider) SetupAndValidateCreateCluster(ctx context.Context,
}
}

if err := p.scrubWorkflowsFromTinkerbell(ctx, p.validator.hardwareConfig.Hardwares, hardware); err != nil {
if err := p.scrubWorkflowsFromTinkerbell(ctx, p.catalogue.Hardware, tinkHardware); err != nil {
return err
}
} else if !p.skipPowerActions {
if err := p.validator.ValidateMachinesPoweredOff(ctx); err != nil {
if err := p.validator.ValidateMachinesPoweredOff(ctx, p.catalogue); err != nil {
return fmt.Errorf("validating machines are powered off: %w", err)
}
}
Expand All @@ -425,7 +432,7 @@ func (p *tinkerbellProvider) SetupAndValidateCreateCluster(ctx context.Context,
return fmt.Errorf("failed validating worker node template config: %v", err)
}

if err := p.validator.ValidateMinimumRequiredTinkerbellHardwareAvailable(tinkerbellClusterSpec.Cluster.Spec); err != nil {
if err := p.validator.ValidateMinimumRequiredTinkerbellHardwareAvailable(tinkerbellClusterSpec.Cluster.Spec, p.catalogue); err != nil {
return fmt.Errorf("minimum hardware not available: %v", err)
}

Expand All @@ -441,7 +448,7 @@ func (p *tinkerbellProvider) SetupAndValidateCreateCluster(ctx context.Context,
}

func (p *tinkerbellProvider) setHardwareStateToProvisining(ctx context.Context) error {
for _, hardware := range p.validator.hardwareConfig.Hardwares {
for _, hardware := range p.catalogue.Hardware {
tinkHardware, err := p.providerTinkClient.GetHardwareByUuid(ctx, hardware.Spec.ID)
if err != nil {
return fmt.Errorf("getting hardware with UUID '%s': %v", hardware.Spec.ID, err)
Expand All @@ -465,20 +472,16 @@ func (p *tinkerbellProvider) setHardwareStateToProvisining(ctx context.Context)
return nil
}

// setMachinesToPXEBoot iterates over all p.validator.hardwareConfig.Bmcs and instructs them to turn off
// and boot from PXE and turn on.
// setMachinesToPXEBoot iterates over all catalogue.BMCs and instructs them to turn off, one-time
// PXE boot, then turn on.
func (p *tinkerbellProvider) setMachinesToPXEBoot(ctx context.Context) error {
// We're reaching into an unexported member of p.validator because of the lazy loading we're doing with
// hardware configuration. This effectively defines a concrete tight coupling between the validator and the
// Tinkerbell construct that desperately needs teething apart.

secrets := make(map[string]corev1.Secret, len(p.validator.hardwareConfig.Secrets))
for _, secret := range p.validator.hardwareConfig.Secrets {
secrets := make(map[string]corev1.Secret, len(p.catalogue.Secrets))
for _, secret := range p.catalogue.Secrets {
secrets[secret.Name] = secret
}

var errs []error
for _, bmc := range p.validator.hardwareConfig.Bmcs {
for _, bmc := range p.catalogue.BMCs {
secret, found := secrets[bmc.Spec.AuthSecretRef.Name]
if !found {
errs = append(errs, fmt.Errorf("could not find bmc secret for '%v'", bmc.Name))
Expand Down
Loading

0 comments on commit 71fae6a

Please sign in to comment.