Use ec2.DescribeInstanceTypes in awsup.GetMachineTypeInfo

This requires passing a cloud object in additional places throughout the validation package and originating mostly from cmd/kops

This means that some kops commands now require valid cloud provider credentials, but I don't think this is an issue because the vast majority of use-cases already require the same cloud provider credentials in order to interact with the state store.
This commit is contained in:
Peter Rifel 2020-06-02 22:59:53 -05:00
parent cebb708fdb
commit bc074e857c
No known key found for this signature in database
GPG Key ID: 30DB43602027D941
18 changed files with 123 additions and 2547 deletions

View File

@ -1231,7 +1231,7 @@ func RunCreateCluster(ctx context.Context, f *util.Factory, out io.Writer, c *Cr
}
strict := false
err = validation.DeepValidate(cluster, instanceGroups, strict)
err = validation.DeepValidate(cluster, instanceGroups, strict, nil)
if err != nil {
return err
}
@ -1255,7 +1255,7 @@ func RunCreateCluster(ctx context.Context, f *util.Factory, out io.Writer, c *Cr
fullInstanceGroups = append(fullInstanceGroups, fullGroup)
}
err = validation.DeepValidate(fullCluster, fullInstanceGroups, true)
err = validation.DeepValidate(fullCluster, fullInstanceGroups, true, nil)
if err != nil {
return err
}

View File

@ -228,7 +228,12 @@ func RunCreateInstanceGroup(ctx context.Context, f *util.Factory, cmd *cobra.Com
return fmt.Errorf("unexpected object type: %T", obj)
}
err = validation.CrossValidateInstanceGroup(group, cluster).ToAggregate()
cloud, err := cloudup.BuildCloud(cluster)
if err != nil {
return err
}
err = validation.CrossValidateInstanceGroup(group, cluster, cloud).ToAggregate()
if err != nil {
return err
}

View File

@ -222,7 +222,12 @@ func RunEditCluster(ctx context.Context, f *util.Factory, cmd *cobra.Command, ar
continue
}
err = validation.DeepValidate(fullCluster, instanceGroups, true)
cloud, err := cloudup.BuildCloud(fullCluster)
if err != nil {
return err
}
err = validation.DeepValidate(fullCluster, instanceGroups, true, cloud)
if err != nil {
results = editResults{
file: file,

View File

@ -173,7 +173,12 @@ func RunEditInstanceGroup(ctx context.Context, f *util.Factory, cmd *cobra.Comma
return err
}
err = validation.CrossValidateInstanceGroup(fullGroup, fullCluster).ToAggregate()
cloud, err := cloudup.BuildCloud(fullCluster)
if err != nil {
return err
}
err = validation.CrossValidateInstanceGroup(fullGroup, fullCluster, cloud).ToAggregate()
if err != nil {
return err
}

View File

@ -452,8 +452,16 @@ func (b *KubeletBuilder) buildKubeletConfigSpec() (*kops.KubeletConfigSpec, erro
instanceTypeName = strings.Split(b.InstanceGroup.Spec.MachineType, ",")[0]
}
region, err := awsup.FindRegion(b.Cluster)
if err != nil {
return nil, err
}
awsCloud, err := awsup.NewAWSCloud(region, nil)
if err != nil {
return nil, err
}
// Get the instance type's detailed information.
instanceType, err := awsup.GetMachineTypeInfo(instanceTypeName)
instanceType, err := awsup.GetMachineTypeInfo(awsCloud, instanceTypeName)
if err != nil {
return nil, err
}

View File

@ -47,6 +47,7 @@ go_test(
deps = [
"//pkg/apis/kops:go_default_library",
"//upup/pkg/fi:go_default_library",
"//upup/pkg/fi/cloudup/awsup:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/intstr:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",

View File

@ -39,19 +39,21 @@ func awsValidateCluster(c *kops.Cluster) field.ErrorList {
return allErrs
}
func awsValidateInstanceGroup(ig *kops.InstanceGroup) field.ErrorList {
func awsValidateInstanceGroup(ig *kops.InstanceGroup, cloud awsup.AWSCloud) field.ErrorList {
allErrs := field.ErrorList{}
allErrs = append(allErrs, awsValidateAdditionalSecurityGroups(field.NewPath("spec", "additionalSecurityGroups"), ig.Spec.AdditionalSecurityGroups)...)
allErrs = append(allErrs, awsValidateMachineType(field.NewPath(ig.GetName(), "spec", "machineType"), ig.Spec.MachineType)...)
if cloud != nil {
allErrs = append(allErrs, awsValidateInstanceType(field.NewPath(ig.GetName(), "spec", "machineType"), ig.Spec.MachineType, cloud)...)
}
allErrs = append(allErrs, awsValidateSpotDurationInMinute(field.NewPath(ig.GetName(), "spec", "spotDurationInMinutes"), ig)...)
allErrs = append(allErrs, awsValidateInstanceInterruptionBehavior(field.NewPath(ig.GetName(), "spec", "instanceInterruptionBehavior"), ig)...)
if ig.Spec.MixedInstancesPolicy != nil {
allErrs = append(allErrs, awsValidateMixedInstancesPolicy(field.NewPath("spec", "mixedInstancesPolicy"), ig.Spec.MixedInstancesPolicy, ig)...)
allErrs = append(allErrs, awsValidateMixedInstancesPolicy(field.NewPath("spec", "mixedInstancesPolicy"), ig.Spec.MixedInstancesPolicy, ig, cloud)...)
}
return allErrs
@ -78,12 +80,11 @@ func awsValidateAdditionalSecurityGroups(fieldPath *field.Path, groups []string)
return allErrs
}
func awsValidateMachineType(fieldPath *field.Path, machineType string) field.ErrorList {
func awsValidateInstanceType(fieldPath *field.Path, instanceType string, cloud awsup.AWSCloud) field.ErrorList {
allErrs := field.ErrorList{}
if machineType != "" {
for _, typ := range strings.Split(machineType, ",") {
if _, err := awsup.GetMachineTypeInfo(typ); err != nil {
if instanceType != "" {
for _, typ := range strings.Split(instanceType, ",") {
if _, err := cloud.DescribeInstanceType(typ); err != nil {
allErrs = append(allErrs, field.Invalid(fieldPath, typ, "machine type specified is invalid"))
}
}
@ -113,13 +114,13 @@ func awsValidateInstanceInterruptionBehavior(fieldPath *field.Path, ig *kops.Ins
}
// awsValidateMixedInstancesPolicy is responsible for validating the user input of a mixed instance policy
func awsValidateMixedInstancesPolicy(path *field.Path, spec *kops.MixedInstancesPolicySpec, ig *kops.InstanceGroup) field.ErrorList {
func awsValidateMixedInstancesPolicy(path *field.Path, spec *kops.MixedInstancesPolicySpec, ig *kops.InstanceGroup, cloud awsup.AWSCloud) field.ErrorList {
var errs field.ErrorList
// @step: check the instances are validate
if cloud != nil {
for i, x := range spec.Instances {
errs = append(errs, awsValidateInstanceType(path.Child("instances").Index(i).Child("instanceType"), x)...)
errs = append(errs, awsValidateInstanceType(path.Child("instances").Index(i).Child("instanceType"), x, cloud)...)
}
}
if spec.OnDemandBase != nil {

View File

@ -20,6 +20,7 @@ import (
"testing"
"k8s.io/kops/upup/pkg/fi"
"k8s.io/kops/upup/pkg/fi/cloudup/awsup"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/kops/pkg/apis/kops"
@ -143,6 +144,7 @@ func TestValidateInstanceGroupSpec(t *testing.T) {
ExpectedErrors: []string{},
},
}
cloud := awsup.BuildMockAWSCloud("us-east-1", "abc")
for _, g := range grid {
ig := &kops.InstanceGroup{
ObjectMeta: v1.ObjectMeta{
@ -150,7 +152,7 @@ func TestValidateInstanceGroupSpec(t *testing.T) {
},
Spec: g.Input,
}
errs := awsValidateInstanceGroup(ig)
errs := awsValidateInstanceGroup(ig, cloud)
testErrors(t, g.Input, errs, g.ExpectedErrors)
}

View File

@ -24,10 +24,11 @@ import (
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/kops/pkg/apis/kops"
"k8s.io/kops/upup/pkg/fi"
"k8s.io/kops/upup/pkg/fi/cloudup/awsup"
)
// ValidateInstanceGroup is responsible for validating the configuration of a instancegroup
func ValidateInstanceGroup(g *kops.InstanceGroup) field.ErrorList {
func ValidateInstanceGroup(g *kops.InstanceGroup, cloud fi.Cloud) field.ErrorList {
allErrs := field.ErrorList{}
if g.ObjectMeta.Name == "" {
@ -114,6 +115,10 @@ func ValidateInstanceGroup(g *kops.InstanceGroup) field.ErrorList {
allErrs = append(allErrs, validateRollingUpdate(g.Spec.RollingUpdate, field.NewPath("spec", "rollingUpdate"), g.Spec.Role == kops.InstanceGroupRoleMaster)...)
}
if awsCloud, ok := cloud.(awsup.AWSCloud); ok {
allErrs = append(allErrs, awsValidateInstanceGroup(g, awsCloud)...)
}
return allErrs
}
@ -151,8 +156,8 @@ func validateVolumeMountSpec(path *field.Path, spec *kops.VolumeMountSpec) field
// CrossValidateInstanceGroup performs validation of the instance group, including that it is consistent with the Cluster
// It calls ValidateInstanceGroup, so all that validation is included.
func CrossValidateInstanceGroup(g *kops.InstanceGroup, cluster *kops.Cluster) field.ErrorList {
allErrs := ValidateInstanceGroup(g)
func CrossValidateInstanceGroup(g *kops.InstanceGroup, cluster *kops.Cluster, cloud fi.Cloud) field.ErrorList {
allErrs := ValidateInstanceGroup(g, cloud)
if g.Spec.Role == kops.InstanceGroupRoleMaster {
allErrs = append(allErrs, ValidateMasterInstanceGroup(g, cluster)...)

View File

@ -507,7 +507,7 @@ func validateSubnetCIDR(networkCIDR *net.IPNet, additionalNetworkCIDRs []*net.IP
}
// DeepValidate is responsible for validating the instancegroups within the cluster spec
func DeepValidate(c *kops.Cluster, groups []*kops.InstanceGroup, strict bool) error {
func DeepValidate(c *kops.Cluster, groups []*kops.InstanceGroup, strict bool, cloud fi.Cloud) error {
if errs := ValidateCluster(c, strict); len(errs) != 0 {
return errs.ToAggregate()
}
@ -535,17 +535,11 @@ func DeepValidate(c *kops.Cluster, groups []*kops.InstanceGroup, strict bool) er
}
for _, g := range groups {
errs := CrossValidateInstanceGroup(g, c)
errs := CrossValidateInstanceGroup(g, c, cloud)
// Additional cloud-specific validation rules,
// such as making sure that identifiers match the expected formats for the given cloud
switch kops.CloudProviderID(c.Spec.CloudProvider) {
case kops.CloudProviderAWS:
errs = append(errs, awsValidateInstanceGroup(g)...)
default:
if len(g.Spec.Volumes) > 0 {
errs = append(errs, field.Forbidden(field.NewPath("spec", "volumes"), "instancegroup volumes are only available with aws at present"))
}
// Additional cloud-specific validation rules
if kops.CloudProviderID(c.Spec.CloudProvider) != kops.CloudProviderAWS && len(g.Spec.Volumes) > 0 {
errs = append(errs, field.Forbidden(field.NewPath("spec", "volumes"), "instancegroup volumes are only available with aws at present"))
}
if len(errs) != 0 {

View File

@ -61,7 +61,7 @@ func NewInstanceGroupMirror(cluster *kopsapi.Cluster, configBase vfs.Path) Insta
}
r.init(kind, configBase.Join("instancegroup"), StoreVersion)
r.validate = func(o runtime.Object) error {
return validation.ValidateInstanceGroup(o.(*kopsapi.InstanceGroup)).ToAggregate()
return validation.ValidateInstanceGroup(o.(*kopsapi.InstanceGroup), nil).ToAggregate()
}
return r
}
@ -80,7 +80,7 @@ func newInstanceGroupVFS(c *VFSClientset, cluster *kopsapi.Cluster) *InstanceGro
}
r.init(kind, c.basePath.Join(clusterName, "instancegroup"), StoreVersion)
r.validate = func(o runtime.Object) error {
return validation.ValidateInstanceGroup(o.(*kopsapi.InstanceGroup)).ToAggregate()
return validation.ValidateInstanceGroup(o.(*kopsapi.InstanceGroup), nil).ToAggregate()
}
return r
}

View File

@ -41,7 +41,7 @@ func UpdateCluster(ctx context.Context, clientset simple.Clientset, cluster *kop
return err
}
err = validation.DeepValidate(fullCluster, instanceGroups, true)
err = validation.DeepValidate(fullCluster, instanceGroups, true, nil)
if err != nil {
return err
}

View File

@ -232,12 +232,17 @@ func (c *ApplyClusterCmd) Run(ctx context.Context) error {
return err
}
err = validation.DeepValidate(c.Cluster, c.InstanceGroups, true)
cluster := c.Cluster
cloud, err := BuildCloud(cluster)
if err != nil {
return err
}
cluster := c.Cluster
err = validation.DeepValidate(c.Cluster, c.InstanceGroups, true, cloud)
if err != nil {
return err
}
if cluster.Spec.KubernetesVersion == "" {
return fmt.Errorf("KubernetesVersion not set")
@ -323,11 +328,6 @@ func (c *ApplyClusterCmd) Run(ctx context.Context) error {
"mirrorSecrets": &fitasks.MirrorSecrets{},
})
cloud, err := BuildCloud(cluster)
if err != nil {
return err
}
region := ""
project := ""
@ -446,8 +446,6 @@ func (c *ApplyClusterCmd) Run(ctx context.Context) error {
if len(sshPublicKeys) > 1 {
return fmt.Errorf("exactly one 'admin' SSH public key can be specified when running with AWS; please delete a key using `kops delete secret`")
}
l.TemplateFunctions["MachineTypeInfo"] = awsup.GetMachineTypeInfo
}
case kops.CloudProviderALI:

View File

@ -26,7 +26,7 @@ import (
// buildEphemeralDevices looks up the machine type and discovery any ephemeral device mappings
func buildEphemeralDevices(cloud awsup.AWSCloud, machineType string) (map[string]*BlockDeviceMapping, error) {
mt, err := awsup.GetMachineTypeInfo(machineType)
mt, err := awsup.GetMachineTypeInfo(cloud, machineType)
if err != nil {
return nil, fmt.Errorf("failed to find instance type details on: %s, error: %s", machineType, err)
}

File diff suppressed because it is too large Load Diff

View File

@ -31,7 +31,7 @@ func TestDeepValidate_OK(t *testing.T) {
var groups []*kopsapi.InstanceGroup
groups = append(groups, buildMinimalMasterInstanceGroup("subnet-us-mock-1a"))
groups = append(groups, buildMinimalNodeInstanceGroup("subnet-us-mock-1a"))
err := validation.DeepValidate(c, groups, true)
err := validation.DeepValidate(c, groups, true, nil)
if err != nil {
t.Fatalf("Expected no error from DeepValidate, got %v", err)
}
@ -172,7 +172,7 @@ func TestDeepValidate_MissingEtcdMember(t *testing.T) {
}
func expectErrorFromDeepValidate(t *testing.T, c *kopsapi.Cluster, groups []*kopsapi.InstanceGroup, message string) {
err := validation.DeepValidate(c, groups, true)
err := validation.DeepValidate(c, groups, true, nil)
if err == nil {
t.Fatalf("Expected error %q from DeepValidate (strict=true), not no error raised", message)
}

View File

@ -47,6 +47,7 @@ const (
defaultALINodeImage = "centos_7_04_64_20G_alibase_201701015.vhd"
)
// TODO: this hardcoded list can be replaced with DescribeInstanceTypes' DedicatedHostsSupported field
var awsDedicatedInstanceExceptions = map[string]bool{
"t2.nano": true,
"t2.micro": true,
@ -60,7 +61,7 @@ var awsDedicatedInstanceExceptions = map[string]bool{
// The InstanceGroup is simpler than the cluster spec, so we just populate in place (like the rest of k8s)
func PopulateInstanceGroupSpec(cluster *kops.Cluster, input *kops.InstanceGroup, channel *kops.Channel) (*kops.InstanceGroup, error) {
var err error
err = validation.ValidateInstanceGroup(input).ToAggregate()
err = validation.ValidateInstanceGroup(input, nil).ToAggregate()
if err != nil {
return nil, err
}

View File

@ -528,7 +528,7 @@ func (_ *Elastigroup) create(cloud awsup.AWSCloud, a, e, changes *Elastigroup) e
return err
}
ephemeralDevices, err := e.buildEphemeralDevices(e.OnDemandInstanceType)
ephemeralDevices, err := e.buildEphemeralDevices(cloud, e.OnDemandInstanceType)
if err != nil {
return err
}
@ -956,7 +956,7 @@ func (_ *Elastigroup) update(cloud awsup.AWSCloud, a, e, changes *Elastigroup) e
return err
}
ephemeralDevices, err := e.buildEphemeralDevices(e.OnDemandInstanceType)
ephemeralDevices, err := e.buildEphemeralDevices(cloud, e.OnDemandInstanceType)
if err != nil {
return err
}
@ -1494,7 +1494,7 @@ func (_ *Elastigroup) RenderTerraform(t *terraform.TerraformTarget, a, e, change
return err
}
ephemeralDevices, err := e.buildEphemeralDevices(e.OnDemandInstanceType)
ephemeralDevices, err := e.buildEphemeralDevices(cloud, e.OnDemandInstanceType)
if err != nil {
return err
}
@ -1638,12 +1638,12 @@ func (e *Elastigroup) buildAutoScaleLabels(labelsMap map[string]string) []*aws.A
return labels
}
func (e *Elastigroup) buildEphemeralDevices(instanceTypeName *string) (map[string]*awstasks.BlockDeviceMapping, error) {
func (e *Elastigroup) buildEphemeralDevices(c awsup.AWSCloud, instanceTypeName *string) (map[string]*awstasks.BlockDeviceMapping, error) {
if instanceTypeName == nil {
return nil, fi.RequiredField("InstanceType")
}
instanceType, err := awsup.GetMachineTypeInfo(*instanceTypeName)
instanceType, err := awsup.GetMachineTypeInfo(c, *instanceTypeName)
if err != nil {
return nil, err
}