-
Notifications
You must be signed in to change notification settings - Fork 1.5k
vSphere: Add IPI-specific validation. #3372
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
openshift-merge-robot
merged 5 commits into
openshift:master
from
patrickdillon:vsphere-ipi-validate
Apr 2, 2020
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
10d127c
vSphere: Refactor VIP validation for Platform.
patrickdillon cd4aee1
vSphere: Add platform validation.
patrickdillon eacf2ba
vSphere: Add IPI-specific platform validation.
patrickdillon a700105
vSphere: Add install config validation unit tests
patrickdillon c7540c0
docs/design/resource_dep: Update dependency graph.
patrickdillon File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| package installconfig | ||
|
|
||
| import ( | ||
| "fmt" | ||
|
|
||
| "github.com/openshift/installer/pkg/asset" | ||
| vsconfig "github.com/openshift/installer/pkg/asset/installconfig/vsphere" | ||
| "github.com/openshift/installer/pkg/types/aws" | ||
| "github.com/openshift/installer/pkg/types/azure" | ||
| "github.com/openshift/installer/pkg/types/baremetal" | ||
| "github.com/openshift/installer/pkg/types/gcp" | ||
| "github.com/openshift/installer/pkg/types/libvirt" | ||
| "github.com/openshift/installer/pkg/types/none" | ||
| "github.com/openshift/installer/pkg/types/openstack" | ||
| "github.com/openshift/installer/pkg/types/ovirt" | ||
| "github.com/openshift/installer/pkg/types/vsphere" | ||
| ) | ||
|
|
||
| // PlatformProvisionCheck is an asset that validates the install-config platform for | ||
| // any requirements specific for provisioning infrastructure. | ||
| type PlatformProvisionCheck struct { | ||
| } | ||
|
|
||
| var _ asset.Asset = (*PlatformProvisionCheck)(nil) | ||
|
|
||
| // Dependencies returns the dependencies for PlatformProvisionCheck | ||
| func (a *PlatformProvisionCheck) Dependencies() []asset.Asset { | ||
| return []asset.Asset{ | ||
| &InstallConfig{}, | ||
| } | ||
| } | ||
|
|
||
| // Generate queries for input from the user. | ||
| func (a *PlatformProvisionCheck) Generate(dependencies asset.Parents) error { | ||
| ic := &InstallConfig{} | ||
| dependencies.Get(ic) | ||
|
|
||
| var err error | ||
| platform := ic.Config.Platform.Name() | ||
| switch platform { | ||
| case vsphere.Name: | ||
| err = vsconfig.ValidateForProvisioning(ic.Config) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| case azure.Name, aws.Name, baremetal.Name, gcp.Name, libvirt.Name, none.Name, openstack.Name, ovirt.Name: | ||
| // no special provisioning requirements to check | ||
| default: | ||
| err = fmt.Errorf("unknown platform type %q", platform) | ||
| } | ||
| return err | ||
| } | ||
|
|
||
| // Name returns the human-friendly name of the asset. | ||
| func (a *PlatformProvisionCheck) Name() string { | ||
| return "Platform Provisioning Check" | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| package vsphere | ||
|
|
||
| import ( | ||
| "github.com/pkg/errors" | ||
| "k8s.io/apimachinery/pkg/util/validation/field" | ||
|
|
||
| "github.com/openshift/installer/pkg/types" | ||
| "github.com/openshift/installer/pkg/types/vsphere/validation" | ||
| ) | ||
|
|
||
| // Validate executes platform-specific validation. | ||
| func Validate(ic *types.InstallConfig) error { | ||
| allErrs := field.ErrorList{} | ||
| if ic.Platform.VSphere == nil { | ||
| return errors.New(field.Required(field.NewPath("platform", "vsphere"), "vSphere validation requires a vSphere platform configuration").Error()) | ||
| } | ||
|
|
||
| allErrs = append(allErrs, validation.ValidatePlatform(ic.Platform.VSphere, field.NewPath("platform").Child("vsphere"))...) | ||
|
|
||
| return allErrs.ToAggregate() | ||
| } | ||
|
|
||
| // ValidateForProvisioning performs platform validation specifically for installer- | ||
| // provisioned infrastructure. In this case, self-hosted networking is a requirement | ||
| // when the installer creates infrastructure for vSphere clusters. | ||
| func ValidateForProvisioning(ic *types.InstallConfig) error { | ||
| allErrs := field.ErrorList{} | ||
| if ic.Platform.VSphere == nil { | ||
| return errors.New(field.Required(field.NewPath("platform", "vsphere"), "vSphere validation requires a vSphere platform configuration").Error()) | ||
| } | ||
|
|
||
| allErrs = append(allErrs, validation.ValidateForProvisioning(ic.Platform.VSphere, field.NewPath("platform").Child("vsphere"))...) | ||
|
|
||
| return allErrs.ToAggregate() | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,106 @@ | ||
| package vsphere | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
|
|
||
| "github.com/openshift/installer/pkg/ipnet" | ||
| "github.com/openshift/installer/pkg/types" | ||
| "github.com/openshift/installer/pkg/types/vsphere" | ||
| ) | ||
|
|
||
| var ( | ||
| validCIDR = "10.0.0.0/16" | ||
| ) | ||
|
|
||
| func validIPIInstallConfig() *types.InstallConfig { | ||
| return &types.InstallConfig{ | ||
| Networking: &types.Networking{ | ||
| MachineNetwork: []types.MachineNetworkEntry{ | ||
| {CIDR: *ipnet.MustParseCIDR(validCIDR)}, | ||
| }, | ||
| }, | ||
| Publish: types.ExternalPublishingStrategy, | ||
| Platform: types.Platform{ | ||
| VSphere: &vsphere.Platform{ | ||
| Cluster: "valid_cluster", | ||
| Datacenter: "valid_dc", | ||
| DefaultDatastore: "valid_ds", | ||
| Network: "valid_network", | ||
| Password: "valid_password", | ||
| Username: "valid_username", | ||
| VCenter: "valid_vcenter", | ||
| APIVIP: "192.168.111.0", | ||
| IngressVIP: "192.168.111.1", | ||
| DNSVIP: "192.168.111.2", | ||
| }, | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| func validUPIInstallConfig() *types.InstallConfig { | ||
| return &types.InstallConfig{ | ||
| Networking: &types.Networking{ | ||
| MachineNetwork: []types.MachineNetworkEntry{ | ||
| {CIDR: *ipnet.MustParseCIDR(validCIDR)}, | ||
| }, | ||
| }, | ||
| Publish: types.ExternalPublishingStrategy, | ||
| Platform: types.Platform{ | ||
| VSphere: &vsphere.Platform{ | ||
| Datacenter: "valid_dc", | ||
| DefaultDatastore: "valid_ds", | ||
| Password: "valid_password", | ||
| Username: "valid_username", | ||
| VCenter: "valid_vcenter", | ||
| }, | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| func TestValidate(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| installConfig *types.InstallConfig | ||
| validationMethod func(*types.InstallConfig) error | ||
| expectErr string | ||
| }{{ | ||
| name: "valid UPI install config", | ||
| installConfig: validUPIInstallConfig(), | ||
| validationMethod: Validate, | ||
| }, { | ||
| name: "valid IPI install config", | ||
| installConfig: validIPIInstallConfig(), | ||
| validationMethod: ValidateForProvisioning, | ||
| }, { | ||
| name: "invalid IPI - no network", | ||
| installConfig: func() *types.InstallConfig { | ||
| c := validIPIInstallConfig() | ||
| c.Platform.VSphere.Network = "" | ||
| return c | ||
| }(), | ||
| validationMethod: ValidateForProvisioning, | ||
| expectErr: `^platform\.vsphere\.network: Required value: must specify the network$`, | ||
| }, { | ||
| name: "invalid IPI - no cluster", | ||
| installConfig: func() *types.InstallConfig { | ||
| c := validIPIInstallConfig() | ||
| c.Platform.VSphere.Cluster = "" | ||
| return c | ||
| }(), | ||
| validationMethod: ValidateForProvisioning, | ||
| expectErr: `^platform\.vsphere\.cluster: Required value: must specify the cluster$`, | ||
| }} | ||
|
|
||
| for _, test := range tests { | ||
| t.Run(test.name, func(t *testing.T) { | ||
| err := test.validationMethod(test.installConfig) | ||
| if test.expectErr == "" { | ||
| assert.NoError(t, err) | ||
| } else { | ||
| assert.Regexp(t, test.expectErr, err.Error()) | ||
| } | ||
| }) | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't this call the
Validate?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so?
Validatewould already be called when loading installconfig asset in cd4aee1