Skip to content
This repository has been archived by the owner on May 7, 2024. It is now read-only.

Commit

Permalink
Merge pull request #606 from giantswarm/use--1-as-default-for-azure-s…
Browse files Browse the repository at this point in the history 
…pot-instances-max-price
  • Loading branch information
@axbarsan
axbarsan committed Feb 9, 2021
2 parents 96fa17f + ad779ca commit 0a2046d
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 57 deletions.
6 changes: 3 additions & 3 deletions commands/create/nodepool/command.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ Examples:
gsctl create nodepool f01r4 --azure-spot-instances --azure-spot-instances-max-price 0.00315
By setting this value to '0', the maximum price will be set
By setting this value to '-1', the maximum price will be set
to the on-demand price of the instance.
`,
Expand Down Expand Up @@ -167,7 +167,7 @@ func initFlags() {
Command.Flags().Int64VarP(&flags.AWSOnDemandBaseCapacity, "aws-on-demand-base-capacity", "", 0, "Number of on-demand instances that this node pool needs to have until spot instances are used (AWS only). Default is 0")
Command.Flags().Int64VarP(&flags.AWSSpotPercentage, "aws-spot-percentage", "", 0, "Percentage of spot instances used once the on-demand base capacity is fullfilled (AWS only). A number of 40 would mean that 60% will be on-demand and 40% will be spot instances.")
Command.Flags().BoolVarP(&flags.AzureSpotInstances, "azure-spot-instances", "", false, "Whether the node pool must use spot instances or on-demand.")
Command.Flags().Float64VarP(&flags.AzureSpotInstancesMaxPrice, "azure-spot-instances-max-price", "", 0, "Max bid hourly price for a single instance. 0 means on-demand price.")
Command.Flags().Float64VarP(&flags.AzureSpotInstancesMaxPrice, "azure-spot-instances-max-price", "", -1, "Max bid hourly price for a single instance. -1 means on-demand price.")
}

// Arguments defines the arguments this command can take into consideration.
Expand Down Expand Up @@ -332,7 +332,7 @@ func verifyPreconditions(args Arguments) error {
return microerror.Mask(errors.NotPercentage)
}
} else if args.Provider == provider.Azure {
if !args.AzureSpotInstances && args.AzureSpotInstancesMaxPrice != 0 {
if !args.AzureSpotInstances && args.AzureSpotInstancesMaxPrice != -1 {
return microerror.Maskf(errors.ConflictingFlagsError, "the flag --azure-spot-instances-max-price cannot be set if spot instances are not enabled.")
}
}
Expand Down
115 changes: 61 additions & 54 deletions commands/create/nodepool/command_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,14 @@ func TestCollectArgs(t *testing.T) {
Command.ParseFlags([]string{"cluster-id", "--name=my-name"})
},
Arguments{
APIEndpoint: mockServer.URL,
AuthToken: "some-token",
ClusterNameOrID: "cluster-id",
Name: "my-name",
Scheme: "giantswarm",
Provider: "aws",
MaxNumOfAvailabilityZones: 3,
APIEndpoint: mockServer.URL,
AuthToken: "some-token",
ClusterNameOrID: "cluster-id",
Name: "my-name",
Scheme: "giantswarm",
Provider: "aws",
MaxNumOfAvailabilityZones: 3,
AzureSpotInstancesMaxPrice: -1,
},
},
{
Expand All @@ -97,18 +98,19 @@ func TestCollectArgs(t *testing.T) {
})
},
Arguments{
APIEndpoint: mockServer.URL,
AuthToken: "some-token",
ClusterNameOrID: "some-cluster-id",
Name: "my-nodepool-name",
Scheme: "giantswarm",
AvailabilityZonesNum: 3,
InstanceType: "instance-type",
ScalingMin: 5,
ScalingMinSet: true,
ScalingMax: 10,
Provider: "aws",
MaxNumOfAvailabilityZones: 3,
APIEndpoint: mockServer.URL,
AuthToken: "some-token",
ClusterNameOrID: "some-cluster-id",
Name: "my-nodepool-name",
Scheme: "giantswarm",
AvailabilityZonesNum: 3,
InstanceType: "instance-type",
ScalingMin: 5,
ScalingMinSet: true,
ScalingMax: 10,
Provider: "aws",
MaxNumOfAvailabilityZones: 3,
AzureSpotInstancesMaxPrice: -1,
},
},
{
Expand All @@ -121,13 +123,14 @@ func TestCollectArgs(t *testing.T) {
})
},
Arguments{
APIEndpoint: mockServer.URL,
AuthToken: "some-token",
ClusterNameOrID: "a-cluster-id",
Scheme: "giantswarm",
AvailabilityZonesList: []string{"myzonea", "myzoneb", "myzonec"},
Provider: "aws",
MaxNumOfAvailabilityZones: 3,
APIEndpoint: mockServer.URL,
AuthToken: "some-token",
ClusterNameOrID: "a-cluster-id",
Scheme: "giantswarm",
AvailabilityZonesList: []string{"myzonea", "myzoneb", "myzonec"},
Provider: "aws",
MaxNumOfAvailabilityZones: 3,
AzureSpotInstancesMaxPrice: -1,
},
},
// Only setting the --nodes-min, but not --nodes-max flag.
Expand All @@ -141,15 +144,16 @@ func TestCollectArgs(t *testing.T) {
})
},
Arguments{
APIEndpoint: mockServer.URL,
AuthToken: "some-token",
ClusterNameOrID: "another-cluster-id",
ScalingMax: 0,
ScalingMin: 5,
ScalingMinSet: true,
Scheme: "giantswarm",
Provider: "aws",
MaxNumOfAvailabilityZones: 3,
APIEndpoint: mockServer.URL,
AuthToken: "some-token",
ClusterNameOrID: "another-cluster-id",
ScalingMax: 0,
ScalingMin: 5,
ScalingMinSet: true,
Scheme: "giantswarm",
Provider: "aws",
MaxNumOfAvailabilityZones: 3,
AzureSpotInstancesMaxPrice: -1,
},
},
// Only setting the --nodes-max, but not --nodes-min flag.
Expand All @@ -163,13 +167,14 @@ func TestCollectArgs(t *testing.T) {
})
},
Arguments{
APIEndpoint: mockServer.URL,
AuthToken: "some-token",
ClusterNameOrID: "another-cluster-id",
ScalingMax: 5,
Scheme: "giantswarm",
Provider: "aws",
MaxNumOfAvailabilityZones: 3,
APIEndpoint: mockServer.URL,
AuthToken: "some-token",
ClusterNameOrID: "another-cluster-id",
ScalingMax: 5,
Scheme: "giantswarm",
Provider: "aws",
MaxNumOfAvailabilityZones: 3,
AzureSpotInstancesMaxPrice: -1,
},
},
// Setting the Azure VM size.
Expand All @@ -183,13 +188,14 @@ func TestCollectArgs(t *testing.T) {
})
},
Arguments{
APIEndpoint: mockServer.URL,
AuthToken: "some-token",
ClusterNameOrID: "another-cluster-id",
VmSize: "something-large",
Scheme: "giantswarm",
Provider: "aws",
MaxNumOfAvailabilityZones: 3,
APIEndpoint: mockServer.URL,
AuthToken: "some-token",
ClusterNameOrID: "another-cluster-id",
VmSize: "something-large",
Scheme: "giantswarm",
Provider: "aws",
MaxNumOfAvailabilityZones: 3,
AzureSpotInstancesMaxPrice: -1,
},
},
}
Expand Down Expand Up @@ -294,11 +300,12 @@ func TestSuccess(t *testing.T) {
// Creation with Azure VM Size.
{
Arguments{
ClusterNameOrID: "cluster-id",
AuthToken: "token",
Name: "my node pool",
VmSize: "my-big-type",
Provider: "azure",
ClusterNameOrID: "cluster-id",
AuthToken: "token",
Name: "my node pool",
VmSize: "my-big-type",
Provider: "azure",
AzureSpotInstancesMaxPrice: -1,
},
`{
"id": "m0ckr",
Expand Down

0 comments on commit 0a2046d

Please sign in to comment.