validation changes for assisted bare metal deployments

Signed-off-by: Doug Hellmann <[email protected]>

Author: dhellmann

enhancements/installer/assisted-installer-bare-metal-validations.md

@@ -0,0 +1,280 @@
+---
+title: assisted-installer-bare-metal-validations
+authors:
+  - "@dhellmann"
+  - "@stbenjam"
+reviewers:
+  - "@romfreiman"
+  - "@yevgeny-shnaidman"
+approvers:
+  - "@hardys"
+  - "@crawford"
+  - "@markmc"
+creation-date: 2020-06-24
+last-updated: 2020-06-26
+status: implementable
+---
+
+# Assisted Installer Bare Metal Validations
+
+## Release Signoff Checklist
+
+- [x] Enhancement is `implementable`
+- [ ] Design details are appropriately documented from clear requirements
+- [ ] Test plan is defined
+- [ ] Graduation criteria for dev preview, tech preview, GA
+- [ ] User-facing documentation is created in [openshift-docs](https://github.com/openshift/openshift-docs/)
+
+## Summary
+
+The assisted installer supports a user-provisioned infrastructure
+installation workflow for bare-metal clusters, but configures the
+installed cluster with certain capabilities that are currently only
+available using the `baremetal` platform in an installer-provisioned
+infrastructure workflow.
+
+This enhancement describes how we can modify the installer to allow
+bare-metal user-provisioned infrastructure clusters, including those
+produced with the assisted installer, to include those
+installer-provisioned infrastructure capabilities.
+
+## Motivation
+
+The [connected-assisted-installer
+enhancement](./connected-assisted-installer.md) describes how an agent
+running on several bare-metal hosts receive a command from the
+assisted installer application to begin the installation of that
+host. One of the first tasks that each host performs upon receiving
+this instruction is to download, from the assisted installer, the
+appropriate Ignition file for that host's role. These ignition files
+are generated by the assisted installer earlier in the process using
+the `openshift-install create ignition-configs` command.
+
+This installer workflow mirrors the [bare-metal user-provisioned
+infrastructure
+workflow](https://github.com/openshift/installer/blob/master/docs/user/metal/install_upi.md)
+in that the the OpenShift installer is used to generate assets for the
+cluster but the `openshift-install create cluster` command is not used
+to manage the deployment. However, the assisted installer does need to
+enable some `baremetal` installer-provisioned infrastructure platform
+features on the installed cluster. Therefore, we need to consider how
+to make those features available while using a user-provisioned
+infrastructure workflow. Currently, the assisted installer is using
+the [workaround of providing dummy
+data](https://github.com/filanov/bm-inventory/blob/54d56712b59d306125ee68d2014c98cf1e995e75/internal/installcfg/installcfg.go#L143-L190)
+for fields that are currently required by the `baremetal` platform but
+that are unused in this use-case.
+
+### Goals
+
+- Allow the use of certain installer-provisioned infrastructure
+  features on bare-metal user-provisioned infrastructure clusters
+- Avoid making any incompatible changes to the bare-metal
+  installer-provisioned infrastructure workflow
+
+### Non-Goals
+
+- Removing or relaxing any validation rules for the
+  installer-provisioned infrastructure installer.
+- Recapitulate the test plans for the assisted installer.
+- Recapitulate the [approach for making parts of the bare metal
+  infrastructure automation optional](./baremetal-provisioning-optional.md).
+
+## Proposal
+
+The [`baremetal.Platform` fields in
+`install-config.yaml`](https://github.com/openshift/installer/blob/master/docs/user/metal/install_ipi.md#install-config)
+can be considered to be configuring three distinct capabilities:
+
+1. Provision a libvirt VM with the bootstrap configuration, and use
+   IPMI/PXE to provision a set of hosts as control plane nodes.
+2. Configure the `baremetal-operator` to run on the installed cluster
+   for day 2 machine-management operations such as power control and
+   automated provisioning of additional workers.
+3. Configure [cluster-hosted control-plane
+   load-balancing](https://github.com/openshift/installer/blob/master/docs/design/baremetal/networking-infrastructure.md)
+   using `keepalived` and `haproxy`.
+
+The proposal is to move where some of the the input validation happens
+to allow the latter 2 capabilities to be configured for
+user-provisioned infrastructure, without requiring configuration that
+is only relevant to the first capability.
+
+### User Stories [optional]
+
+#### Story 1
+
+As a developer building the assisted installer tools, I want to be
+able to run the OpenShift installer to generate assets for a bare
+metal cluster without invoking validations that only apply to the
+installer-provisioned infrastructure workflows so I can use the assets
+in user-provisioned workflows.
+
+#### Story 2
+
+As a developer of the OpenShift installer, I want to be able to add
+validation rules for bare metal clusters correctly based on whether
+the cluster is being built using the installer-provisioned
+infrastructure workflow or the user-provisioned workflow so I can
+apply only the rules necessary for each case.
+
+### Implementation Details/Notes/Constraints [optional]
+
+All of the validation is currently performed during early phases of
+the installer for the `InstallConfig` target. The user-provisioned
+workflow used by the assisted installer does not run the phase for the
+`Cluster` target (it only ever runs up to the `IgnitionConfigs`
+target), so if we move some of the validation into the later phase we
+will have a better separation of the parts needed to build the assets
+for the user-provisioned workflow and the assisted installer from the
+parts needed to run the installer for installer-provisioned
+infrastructure for bare metal. A secondary benefit is that the
+validation for the `baremetal` platform will more closely match the
+validation for cloud platforms.
+
+The install config inputs are currently validated as part of
+generating the `InstallConfig` asset in
+[InstallConfig.finish()](https://github.com/openshift/installer/blob/master/pkg/asset/installconfig/installconfig.go#L141),
+which invokes some logic that applies to all platforms in
+[ValidateInstallConfig()](https://github.com/openshift/installer/blob/master/pkg/types/validation/installconfig.go#L40)
+before running the platform-specific logic from
+[validatePlatform()](https://github.com/openshift/installer/blob/master/pkg/types/validation/installconfig.go#L341). For
+the `baremetal` platform that results in calling
+[ValidatePlatform()](https://github.com/openshift/installer/blob/master/pkg/types/baremetal/validation/platform.go#L218).
+
+The
+[PlatformProvisionCheck](https://github.com/openshift/installer/blob/master/pkg/asset/installconfig/platformprovisioncheck.go)
+and
+[PlatformCredsCheck](https://github.com/openshift/installer/blob/master/pkg/asset/installconfig/platformcredscheck.go)
+assets are both dependencies for the `Cluster` asset, which means they
+are evaluated later in the installation process. Both already have
+platform-specific logic, although not for the `baremetal` platform.
+
+The assisted installer does not need the `bootstrapProvisioningIP`,
+`provisioningNetworkCIDR`, `provisioningDHCPRange`, or
+`provisioningNetworkInterface` fields. The values in those fields are
+used to configure the provisioning tools used for the
+installer-provisioned infrastructure workflow, so those validation
+checks could move to a new function in the
+`pkg/types/baremetal/validation` package that can be invoked from
+[PlatformProvisionCheck.Generate()](https://github.com/openshift/installer/blob/master/pkg/asset/installconfig/platformprovisioncheck.go#L37).
+
+There are two validation rules for the `clusterOSImage` and
+`bootstrapOSImage` fields, to ensure the value is a valid URL and to
+ensure the URL exists. Those checks can be split between the two
+phases, so that the URL format validation is done in
+`ValidateInstallConfig()` and the existence check is moved to
+`PlatformProvisionCheck`.
+
+The assisted installer does not need the `libvirtURI` or host BMC
+credentials, so the checks for those values could move to a new public
+function in the `pkg/types/baremetal/validation` package that can be
+invoked from
+[PlatformCredsCheck.Generate()](https://github.com/openshift/installer/blob/master/pkg/asset/installconfig/platformcredscheck.go#L60).
+
+Changing the BMC credential validation in the installer is only part
+of the work required to make `BareMetalHost` resources work without
+credentials. The `baremetal-operator` also [needs to be
+updated](https://github.com/metal3-io/baremetal-operator/pull/563) to
+not treat the missing credentials as an error.
+
+### Risks and Mitigations
+
+Moving some of the validation steps means that invalid data will be
+caught later in the process. Most users will run the `create cluster`
+command directly, so they will not see a significant change. When the
+installer runs under Hive, or if the user runs the phases individually
+so they can inject extra manifests, the `create cluster` step may
+happen much later in the process. Bad data will still be caught before
+the bootstrap VM is launched, though, so it should be relatively quick
+in the overall installation process.
+
+There are no security implications for this change.
+
+## Design Details
+
+### Test Plan
+
+Besides the existing and new unit tests, the existing e2e tests for
+the `baremetal` platform will continue to test the full set of
+validation rules. The test suite for the assisted installer will
+verify that no extra validation rules are applied.
+
+### Graduation Criteria
+
+**Note:** *Section not required until targeted at a release.*
+
+Define graduation milestones.
+
+These may be defined in terms of API maturity, or as something else. Initial proposal
+should keep this high-level with a focus on what signals will be looked at to
+determine graduation.
+
+Consider the following in developing the graduation criteria for this
+enhancement:
+- Maturity levels - `Dev Preview`, `Tech Preview`, `GA`
+- Deprecation
+
+Clearly define what graduation means.
+
+#### Examples
+
+These are generalized examples to consider, in addition to the aforementioned
+[maturity levels][maturity-levels].
+
+##### Dev Preview -> Tech Preview
+
+- Ability to utilize the enhancement end to end
+- End user documentation, relative API stability
+- Sufficient test coverage
+- Gather feedback from users rather than just developers
+
+##### Tech Preview -> GA 
+
+- More testing (upgrade, downgrade, scale)
+- Sufficient time for feedback
+- Available by default
+
+**For non-optional features moving to GA, the graduation criteria must include
+end to end tests.**
+
+## Implementation History
+
+Major milestones in the life cycle of a proposal should be tracked in `Implementation
+History`.
+
+## Alternatives
+
+### Add a new platform
+
+We could add a new platform to the installer to provide a clear home
+for all of the logic that is specific to the assisted installer. This
+would be a more invasive change, and might end up being confusing
+since the installer would either still need to configure the cluster
+with the `baremetal` platform or the `machine-api-operator` would need
+to understand a new platform and configure the correct machine API
+implementation.
+
+### Add flag(s) to install-config.yml
+
+We could add a flag or other input field to `install-config.yml` to
+control the validation behavior. For example,
+<https://github.com/openshift/installer/pull/3794> proposes a list of
+validation rules to skip. There are two reasons not to do this. First,
+it would expose the validation rules through the installer's public
+API, even though we would only want the assisted installer to use the
+feature. Second, it places the burden of managing the set of
+validations on the wrapper in the assisted installer, instead of on
+the author of validation rules in the installer source code. We know
+which validations should apply in each case, so there is no need to
+make the validations configurable.
+
+### Add an environment variable
+
+An environment variable is a hidden API control for the installer,
+which makes using it and debugging its use more difficult. It also
+implies that there may be checks for the variable in several places in
+the code where the existing validation rules are, which spreads the
+knowledge of which validation rules apply in different scenarios
+throughout the installer code base, instead of using the existing
+dependency structure to manage them.