Skip to content

Commit

Permalink
Merge pull request #7252 from spotinst/feature-spotinst-hybrid
Browse files Browse the repository at this point in the history
Spotinst: New hybrid integration mode
  • Loading branch information
k8s-ci-robot authored May 23, 2020
2 parents a31abc8 + 04d83c6 commit af2d6b1
Show file tree
Hide file tree
Showing 8 changed files with 93 additions and 23 deletions.
2 changes: 2 additions & 0 deletions pkg/featureflag/featureflag.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ var (
Spotinst = New("Spotinst", Bool(false))
// SpotinstOcean toggles the use of Spotinst Ocean instance group implementation.
SpotinstOcean = New("SpotinstOcean", Bool(false))
// SpotinstHybrid toggles between hybrid and full instance group implementations.
SpotinstHybrid = New("SpotinstHybrid", Bool(false))
// SpotinstController toggles the installation of the Spotinst controller addon.
SpotinstController = New("SpotinstController", Bool(true))
// VPCSkipEnableDNSSupport if set will make that a VPC does not need DNSSupport enabled.
Expand Down
1 change: 1 addition & 0 deletions pkg/model/awsmodel/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ go_library(
"//pkg/featureflag:go_default_library",
"//pkg/model:go_default_library",
"//pkg/model/defaults:go_default_library",
"//pkg/model/spotinstmodel:go_default_library",
"//upup/pkg/fi:go_default_library",
"//upup/pkg/fi/cloudup/awstasks:go_default_library",
"//upup/pkg/fi/fitasks:go_default_library",
Expand Down
10 changes: 10 additions & 0 deletions pkg/model/awsmodel/autoscalinggroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,12 @@ import (
"fmt"
"strings"

"k8s.io/klog"
"k8s.io/kops/pkg/apis/kops"
"k8s.io/kops/pkg/featureflag"
"k8s.io/kops/pkg/model"
"k8s.io/kops/pkg/model/defaults"
"k8s.io/kops/pkg/model/spotinstmodel"
"k8s.io/kops/upup/pkg/fi"
"k8s.io/kops/upup/pkg/fi/cloudup/awstasks"

Expand Down Expand Up @@ -54,6 +57,13 @@ func (b *AutoscalingGroupModelBuilder) Build(c *fi.ModelBuilderContext) error {
for _, ig := range b.InstanceGroups {
name := b.AutoscalingGroupName(ig)

if featureflag.SpotinstHybrid.Enabled() {
if spotinstmodel.HybridInstanceGroup(ig) {
klog.V(2).Infof("Skipping instance group: %q", name)
continue
}
}

// @check if his instancegroup is backed by a fleet and override with a launch template
task, err := func() (fi.Task, error) {
switch UseLaunchTemplate(ig) {
Expand Down
1 change: 0 additions & 1 deletion pkg/model/spotinstmodel/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ go_library(
"//pkg/apis/kops:go_default_library",
"//pkg/featureflag:go_default_library",
"//pkg/model:go_default_library",
"//pkg/model/awsmodel:go_default_library",
"//pkg/model/defaults:go_default_library",
"//upup/pkg/fi:go_default_library",
"//upup/pkg/fi/cloudup/awstasks:go_default_library",
Expand Down
33 changes: 29 additions & 4 deletions pkg/model/spotinstmodel/instance_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,18 @@ import (
"k8s.io/kops/pkg/apis/kops"
"k8s.io/kops/pkg/featureflag"
"k8s.io/kops/pkg/model"
"k8s.io/kops/pkg/model/awsmodel"
"k8s.io/kops/pkg/model/defaults"
"k8s.io/kops/upup/pkg/fi"
"k8s.io/kops/upup/pkg/fi/cloudup/awstasks"
"k8s.io/kops/upup/pkg/fi/cloudup/spotinsttasks"
)

const (
// InstanceGroupLabelHybrid is the metadata label used on the instance group
// to specify that the Spotinst provider should be used to upon creation.
InstanceGroupLabelHybrid = "spotinst.io/hybrid"
InstanceGroupLabelManaged = "spotinst.io/managed" // for backward compatibility

// InstanceGroupLabelSpotPercentage is the metadata label used on the
// instance group to specify the percentage of Spot instances that
// should spin up from the target capacity.
Expand Down Expand Up @@ -101,7 +105,7 @@ const (

// InstanceGroupModelBuilder configures InstanceGroup objects
type InstanceGroupModelBuilder struct {
*awsmodel.AWSModelContext
*model.KopsModelContext

BootstrapScript *model.BootstrapScript
Lifecycle *fi.Lifecycle
Expand All @@ -115,8 +119,16 @@ func (b *InstanceGroupModelBuilder) Build(c *fi.ModelBuilderContext) error {
var err error

for _, ig := range b.InstanceGroups {
klog.V(2).Infof("Building instance group: %q", b.AutoscalingGroupName(ig))
name := b.AutoscalingGroupName(ig)

if featureflag.SpotinstHybrid.Enabled() {
if !HybridInstanceGroup(ig) {
klog.V(2).Infof("Skipping instance group: %q", name)
continue
}
}

klog.V(2).Infof("Building instance group: %q", name)
switch ig.Spec.Role {

// Create both Master and Bastion instance groups as Elastigroups.
Expand Down Expand Up @@ -642,7 +654,7 @@ func (b *InstanceGroupModelBuilder) buildRootVolumeOpts(ig *kops.InstanceGroup)
{
typ := fi.StringValue(ig.Spec.RootVolumeType)
if typ == "" {
typ = awsmodel.DefaultVolumeType
typ = "gp2"
}
opts.Type = fi.String(typ)
}
Expand Down Expand Up @@ -913,3 +925,16 @@ func defaultSpotPercentage(ig *kops.InstanceGroup) *float64 {

return &percentage
}

// HybridInstanceGroup indicates whether the instance group labeled with
// a metadata label `spotinst.io/hybrid` which means the Spotinst provider
// should be used to upon creation if the `SpotinstHybrid` feature flag is on.
func HybridInstanceGroup(ig *kops.InstanceGroup) bool {
v, ok := ig.ObjectMeta.Labels[InstanceGroupLabelHybrid]
if !ok {
v = ig.ObjectMeta.Labels[InstanceGroupLabelManaged]
}

hybrid, _ := strconv.ParseBool(v)
return hybrid
}
4 changes: 1 addition & 3 deletions pkg/resources/aws/aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ func ListResourcesAWS(cloud awsup.AWSCloud, clusterName string) (map[string]*res
//ListCloudFormationStacks,

// EC2
ListAutoScalingGroups,
ListInstances,
ListKeypairs,
ListSecurityGroups,
Expand All @@ -86,9 +87,6 @@ func ListResourcesAWS(cloud awsup.AWSCloud, clusterName string) (map[string]*res
if featureflag.Spotinst.Enabled() {
// Spotinst resources
listFunctions = append(listFunctions, ListSpotinstResources)
} else {
// AutoScaling Groups
listFunctions = append(listFunctions, ListAutoScalingGroups)
}

for _, fn := range listFunctions {
Expand Down
34 changes: 21 additions & 13 deletions upup/pkg/fi/cloudup/apply_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -741,24 +741,32 @@ func (c *ApplyClusterCmd) Run(ctx context.Context) error {
}
switch kops.CloudProviderID(cluster.Spec.CloudProvider) {
case kops.CloudProviderAWS:
awsModelContext := &awsmodel.AWSModelContext{
KopsModelContext: modelContext,
}
{
awsModelContext := &awsmodel.AWSModelContext{
KopsModelContext: modelContext,
}

if featureflag.Spotinst.Enabled() {
l.Builders = append(l.Builders, &spotinstmodel.InstanceGroupModelBuilder{
awsModelBuilder := &awsmodel.AutoscalingGroupModelBuilder{
AWSModelContext: awsModelContext,
BootstrapScript: bootstrapScriptBuilder,
Lifecycle: &clusterLifecycle,
SecurityLifecycle: &securityLifecycle,
})
} else {
l.Builders = append(l.Builders, &awsmodel.AutoscalingGroupModelBuilder{
AWSModelContext: awsModelContext,
BootstrapScript: bootstrapScriptBuilder,
Lifecycle: &clusterLifecycle,
SecurityLifecycle: &securityLifecycle,
})
}

if featureflag.Spotinst.Enabled() {
l.Builders = append(l.Builders, &spotinstmodel.InstanceGroupModelBuilder{
KopsModelContext: modelContext,
BootstrapScript: bootstrapScriptBuilder,
Lifecycle: &clusterLifecycle,
SecurityLifecycle: &securityLifecycle,
})

if featureflag.SpotinstHybrid.Enabled() {
l.Builders = append(l.Builders, awsModelBuilder)
}
} else {
l.Builders = append(l.Builders, awsModelBuilder)
}
}
case kops.CloudProviderDO:
doModelContext := &domodel.DOModelContext{
Expand Down
31 changes: 29 additions & 2 deletions upup/pkg/fi/cloudup/awsup/aws_cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,12 @@ func NewEC2Filter(name string, values ...string) *ec2.Filter {
// DeleteGroup deletes an aws autoscaling group
func (c *awsCloudImplementation) DeleteGroup(g *cloudinstances.CloudInstanceGroup) error {
if c.spotinst != nil {
if featureflag.SpotinstHybrid.Enabled() {
if _, ok := g.Raw.(*autoscaling.Group); ok {
return deleteGroup(c, g)
}
}

return spotinst.DeleteInstanceGroup(c.spotinst, g)
}

Expand Down Expand Up @@ -425,6 +431,12 @@ func deleteGroup(c AWSCloud, g *cloudinstances.CloudInstanceGroup) error {
// DeleteInstance deletes an aws instance
func (c *awsCloudImplementation) DeleteInstance(i *cloudinstances.CloudInstanceGroupMember) error {
if c.spotinst != nil {
if featureflag.SpotinstHybrid.Enabled() {
if _, ok := i.CloudInstanceGroup.Raw.(*autoscaling.Group); ok {
return deleteInstance(c, i)
}
}

return spotinst.DeleteInstance(c.spotinst, i)
}

Expand Down Expand Up @@ -490,8 +502,23 @@ func detachInstance(c AWSCloud, i *cloudinstances.CloudInstanceGroupMember) erro
// GetCloudGroups returns a groups of instances that back a kops instance groups
func (c *awsCloudImplementation) GetCloudGroups(cluster *kops.Cluster, instancegroups []*kops.InstanceGroup, warnUnmatched bool, nodes []v1.Node) (map[string]*cloudinstances.CloudInstanceGroup, error) {
if c.spotinst != nil {
return spotinst.GetCloudGroups(c.spotinst, cluster,
instancegroups, warnUnmatched, nodes)
sgroups, err := spotinst.GetCloudGroups(c.spotinst, cluster, instancegroups, warnUnmatched, nodes)
if err != nil {
return nil, err
}

if featureflag.SpotinstHybrid.Enabled() {
agroups, err := getCloudGroups(c, cluster, instancegroups, warnUnmatched, nodes)
if err != nil {
return nil, err
}

for name, group := range agroups {
sgroups[name] = group
}
}

return sgroups, nil
}

return getCloudGroups(c, cluster, instancegroups, warnUnmatched, nodes)
Expand Down

0 comments on commit af2d6b1

Please sign in to comment.