Skip to content
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.

Sign up for GitHub

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

Try deleting machine pool user data file from S3 when pruning an old launch template version, add S3 storage tests (port to release-2.4) #606

Merged
merged 2 commits into from
Jul 24, 2024
Merged

Try deleting machine pool user data file from S3 when pruning an old launch template version, add S3 storage tests (port to release-2.4) #606

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
abc004d
Try deleting machine pool user data file from S3 when pruning an old …
AndiDog Jul 22, 2024
67deb62
Fix test
AndiDog Jul 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions api/v1beta2/tags.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,12 @@ const (
// of the bootstrap secret that was used to create the user data for the latest launch
// template version.
LaunchTemplateBootstrapDataSecret = NameAWSProviderPrefix + "bootstrap-data-secret"

// LaunchTemplateBootstrapDataHash is the tag we use to store the hash of the raw bootstrap data.
// If bootstrap data is stored in S3, this hash relates to that data, not to the EC2 instance
// user data which only references the S3 object. We store this tag on launch template versions
// so that S3 bootstrap data objects can be deleted when they get outdated.
LaunchTemplateBootstrapDataHash = NameAWSProviderPrefix + "bootstrap-data-hash"
)

// ClusterTagKey generates the key for resources associated with a cluster.
Expand Down
2 changes: 1 addition & 1 deletion exp/controllers/awsmachinepool_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@
}

launchTemplateID := machinePoolScope.AWSMachinePool.Status.LaunchTemplateID
launchTemplate, _, _, err := ec2Svc.GetLaunchTemplate(machinePoolScope.LaunchTemplateName())
launchTemplate, _, _, _, err := ec2Svc.GetLaunchTemplate(machinePoolScope.LaunchTemplateName())

Check failure on line 416 in exp/controllers/awsmachinepool_controller.go

View workflow job for this annotation

GitHub Actions / lint 

declaration has 3 blank identifiers (dogsled)
if err != nil {
return err
}
Expand Down
202 changes: 188 additions & 14 deletions exp/controllers/awsmachinepool_controller_test.go
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion exp/controllers/awsmanagedmachinepool_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@
runPostLaunchTemplateUpdateOperation := func() error {
return nil
}
var objectStoreSvc services.ObjectStoreInterface = nil // no S3 bucket support for `AWSManagedControlPlane` yet

Check failure on line 224 in exp/controllers/awsmanagedmachinepool_controller.go

View workflow job for this annotation

GitHub Actions / lint 

var-declaration: should drop = nil from declaration of var objectStoreSvc; it is the zero value (revive)

Check warning on line 224 in exp/controllers/awsmanagedmachinepool_controller.go

View workflow job for this annotation

GitHub Actions / lint 

var-declaration: should drop = nil from declaration of var objectStoreSvc; it is the zero value (revive)
res, err := reconSvc.ReconcileLaunchTemplate(machinePoolScope, machinePoolScope, s3Scope, ec2svc, objectStoreSvc, canStartInstanceRefresh, cancelInstanceRefresh, runPostLaunchTemplateUpdateOperation)
if err != nil {
r.Recorder.Eventf(machinePoolScope.ManagedMachinePool, corev1.EventTypeWarning, "FailedLaunchTemplateReconcile", "Failed to reconcile launch template: %v", err)
Expand Down Expand Up @@ -269,7 +269,7 @@

if machinePoolScope.ManagedMachinePool.Spec.AWSLaunchTemplate != nil {
launchTemplateID := machinePoolScope.ManagedMachinePool.Status.LaunchTemplateID
launchTemplate, _, _, err := ec2Svc.GetLaunchTemplate(machinePoolScope.LaunchTemplateName())
launchTemplate, _, _, _, err := ec2Svc.GetLaunchTemplate(machinePoolScope.LaunchTemplateName())
if err != nil {
return err
}
Expand Down
116 changes: 79 additions & 37 deletions pkg/cloud/services/ec2/launchtemplate.go
View file
Open in desktop

Large diffs are not rendered by default.

69 changes: 43 additions & 26 deletions pkg/cloud/services/ec2/launchtemplate_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,19 @@

var testUserDataHash = userdata.ComputeHash([]byte(testUserData))

func defaultEC2AndUserDataSecretKeyTags(name string, clusterName string, userDataSecretKey types.NamespacedName) []*ec2.Tag {
var testBootstrapData = []byte("different from testUserData since bootstrap data may be in S3 while EC2 user data points to that S3 object")
var testBootstrapDataHash = userdata.ComputeHash(testBootstrapData)

func defaultEC2AndDataTags(name string, clusterName string, userDataSecretKey types.NamespacedName, bootstrapDataHash string) []*ec2.Tag {
tags := defaultEC2Tags(name, clusterName)
tags = append(tags, &ec2.Tag{

Check failure on line 88 in pkg/cloud/services/ec2/launchtemplate_test.go

View workflow job for this annotation

GitHub Actions / lint 

appendCombine: can combine chain of 2 appends into one (gocritic)
Key: aws.String(infrav1.LaunchTemplateBootstrapDataSecret),
Value: aws.String(userDataSecretKey.String()),
})
tags = append(tags, &ec2.Tag{
Key: aws.String(infrav1.LaunchTemplateBootstrapDataHash),
Value: aws.String(bootstrapDataHash),
})
sortTags(tags)
return tags
}
Expand Down Expand Up @@ -295,20 +302,21 @@
tc.expect(mockEC2Client.EXPECT())
}

launchTemplate, userData, _, err := s.GetLaunchTemplate(tc.launchTemplateName)
launchTemplate, userData, _, _, err := s.GetLaunchTemplate(tc.launchTemplateName)
tc.check(g, launchTemplate, userData, err)
})
}
}

func TestServiceSDKToLaunchTemplate(t *testing.T) {
tests := []struct {
name string
input *ec2.LaunchTemplateVersion
wantLT *expinfrav1.AWSLaunchTemplate
wantHash string
wantDataSecretKey *types.NamespacedName
wantErr bool
name string
input *ec2.LaunchTemplateVersion
wantLT *expinfrav1.AWSLaunchTemplate
wantUserDataHash string
wantDataSecretKey *types.NamespacedName
wantBootstrapDataHash *string
wantErr bool
}{
{
name: "lots of input",
Expand Down Expand Up @@ -350,8 +358,9 @@
SSHKeyName: aws.String("foo-keyname"),
VersionNumber: aws.Int64(1),
},
wantHash: testUserDataHash,
wantDataSecretKey: nil, // respective tag is not given
wantUserDataHash: testUserDataHash,
wantDataSecretKey: nil, // respective tag is not given
wantBootstrapDataHash: nil, // respective tag is not given
},
{
name: "tag of bootstrap secret",
Expand Down Expand Up @@ -388,6 +397,10 @@
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/bootstrap-data-secret"),
Value: aws.String("bootstrap-secret-ns/bootstrap-secret"),
},
{
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/bootstrap-data-hash"),
Value: aws.String(testBootstrapDataHash),
},
},
},
},
Expand All @@ -404,26 +417,29 @@
SSHKeyName: aws.String("foo-keyname"),
VersionNumber: aws.Int64(1),
},
wantHash: testUserDataHash,
wantDataSecretKey: &types.NamespacedName{Namespace: "bootstrap-secret-ns", Name: "bootstrap-secret"},
wantUserDataHash: testUserDataHash,
wantDataSecretKey: &types.NamespacedName{Namespace: "bootstrap-secret-ns", Name: "bootstrap-secret"},
wantBootstrapDataHash: &testBootstrapDataHash,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
g := NewWithT(t)
s := &Service{}
gotLT, gotHash, gotDataSecretKey, err := s.SDKToLaunchTemplate(tt.input)
gotLT, gotUserDataHash, gotDataSecretKey, gotBootstrapDataHash, err := s.SDKToLaunchTemplate(tt.input)
if (err != nil) != tt.wantErr {
t.Fatalf("error mismatch: got %v, wantErr %v", err, tt.wantErr)
}
if !cmp.Equal(gotLT, tt.wantLT) {
t.Fatalf("launchTemplate mismatch: got %v, want %v", gotLT, tt.wantLT)
}
if !cmp.Equal(gotHash, tt.wantHash) {
t.Fatalf("userDataHash mismatch: got %v, want %v", gotHash, tt.wantHash)
if !cmp.Equal(gotUserDataHash, tt.wantUserDataHash) {
t.Fatalf("userDataHash mismatch: got %v, want %v", gotUserDataHash, tt.wantUserDataHash)
}
if !cmp.Equal(gotDataSecretKey, tt.wantDataSecretKey) {
t.Fatalf("userDataSecretKey mismatch: got %v, want %v", gotDataSecretKey, tt.wantDataSecretKey)
}
g.Expect(gotBootstrapDataHash).To(Equal(tt.wantBootstrapDataHash))
})
}
}
Expand Down Expand Up @@ -845,7 +861,7 @@
TagSpecifications: []*ec2.LaunchTemplateTagSpecificationRequest{
{
ResourceType: aws.String(ec2.ResourceTypeInstance),
Tags: defaultEC2AndUserDataSecretKeyTags("aws-mp-name", "cluster-name", userDataSecretKey),
Tags: defaultEC2AndDataTags("aws-mp-name", "cluster-name", userDataSecretKey, testBootstrapDataHash),
},
{
ResourceType: aws.String(ec2.ResourceTypeVolume),
Expand Down Expand Up @@ -905,7 +921,7 @@
TagSpecifications: []*ec2.LaunchTemplateTagSpecificationRequest{
{
ResourceType: aws.String(ec2.ResourceTypeInstance),
Tags: defaultEC2AndUserDataSecretKeyTags("aws-mp-name", "cluster-name", userDataSecretKey),
Tags: defaultEC2AndDataTags("aws-mp-name", "cluster-name", userDataSecretKey, testBootstrapDataHash),
},
{
ResourceType: aws.String(ec2.ResourceTypeVolume),
Expand Down Expand Up @@ -967,7 +983,7 @@
TagSpecifications: []*ec2.LaunchTemplateTagSpecificationRequest{
{
ResourceType: aws.String(ec2.ResourceTypeInstance),
Tags: defaultEC2AndUserDataSecretKeyTags("aws-mp-name", "cluster-name", userDataSecretKey),
Tags: defaultEC2AndDataTags("aws-mp-name", "cluster-name", userDataSecretKey, testBootstrapDataHash),
},
{
ResourceType: aws.String(ec2.ResourceTypeVolume),
Expand Down Expand Up @@ -1022,7 +1038,7 @@
tc.expect(g, mockEC2Client.EXPECT())
}

launchTemplate, err := s.CreateLaunchTemplate(ms, aws.String("imageID"), userDataSecretKey, userData)
launchTemplate, err := s.CreateLaunchTemplate(ms, aws.String("imageID"), userDataSecretKey, userData, testBootstrapDataHash)
tc.check(g, launchTemplate, err)
})
}
Expand Down Expand Up @@ -1050,7 +1066,7 @@
Namespace: "bootstrap-secret-ns",
Name: "bootstrap-secret",
}
launchTemplate, err := s.CreateLaunchTemplate(ms, aws.String("imageID"), userDataSecretKey, nil)
launchTemplate, err := s.CreateLaunchTemplate(ms, aws.String("imageID"), userDataSecretKey, nil, "")
g.Expect(err).To(HaveOccurred())
g.Expect(launchTemplate).Should(BeEmpty())
})
Expand Down Expand Up @@ -1104,7 +1120,7 @@
TagSpecifications: []*ec2.LaunchTemplateTagSpecificationRequest{
{
ResourceType: aws.String(ec2.ResourceTypeInstance),
Tags: defaultEC2AndUserDataSecretKeyTags("aws-mp-name", "cluster-name", userDataSecretKey),
Tags: defaultEC2AndDataTags("aws-mp-name", "cluster-name", userDataSecretKey, testBootstrapDataHash),
},
{
ResourceType: aws.String(ec2.ResourceTypeVolume),
Expand Down Expand Up @@ -1155,7 +1171,7 @@
TagSpecifications: []*ec2.LaunchTemplateTagSpecificationRequest{
{
ResourceType: aws.String(ec2.ResourceTypeInstance),
Tags: defaultEC2AndUserDataSecretKeyTags("aws-mp-name", "cluster-name", userDataSecretKey),
Tags: defaultEC2AndDataTags("aws-mp-name", "cluster-name", userDataSecretKey, testBootstrapDataHash),
},
{
ResourceType: aws.String(ec2.ResourceTypeVolume),
Expand Down Expand Up @@ -1202,10 +1218,10 @@
tc.expect(mockEC2Client.EXPECT())
}
if tc.wantErr {
g.Expect(s.CreateLaunchTemplateVersion("launch-template-id", ms, aws.String("imageID"), userDataSecretKey, userData)).To(HaveOccurred())
g.Expect(s.CreateLaunchTemplateVersion("launch-template-id", ms, aws.String("imageID"), userDataSecretKey, userData, testBootstrapDataHash)).To(HaveOccurred())
return
}
g.Expect(s.CreateLaunchTemplateVersion("launch-template-id", ms, aws.String("imageID"), userDataSecretKey, userData)).NotTo(HaveOccurred())
g.Expect(s.CreateLaunchTemplateVersion("launch-template-id", ms, aws.String("imageID"), userDataSecretKey, userData, testBootstrapDataHash)).NotTo(HaveOccurred())
})
}
}
Expand All @@ -1218,6 +1234,7 @@
Namespace: "bootstrap-secret-ns",
Name: "bootstrap-secret",
}
bootstrapDataHash := userdata.ComputeHash([]byte("shell-script"))
testCases := []struct {
name string
check func(g *WithT, m []*ec2.LaunchTemplateTagSpecificationRequest)
Expand All @@ -1228,7 +1245,7 @@
expected := []*ec2.LaunchTemplateTagSpecificationRequest{
{
ResourceType: aws.String(ec2.ResourceTypeInstance),
Tags: defaultEC2AndUserDataSecretKeyTags("aws-mp-name", "cluster-name", userDataSecretKey),
Tags: defaultEC2AndDataTags("aws-mp-name", "cluster-name", userDataSecretKey, bootstrapDataHash),
},
{
ResourceType: aws.String(ec2.ResourceTypeVolume),
Expand Down Expand Up @@ -1258,7 +1275,7 @@
g.Expect(err).NotTo(HaveOccurred())

s := NewService(cs)
tc.check(g, s.buildLaunchTemplateTagSpecificationRequest(ms, userDataSecretKey))
tc.check(g, s.buildLaunchTemplateTagSpecificationRequest(ms, userDataSecretKey, bootstrapDataHash))
})
}
}
Expand Down
10 changes: 6 additions & 4 deletions pkg/cloud/services/interfaces.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ limitations under the License.
package services

import (
"github.com/aws/aws-sdk-go/service/ec2"
apimachinerytypes "k8s.io/apimachinery/pkg/types"
ctrl "sigs.k8s.io/controller-runtime"

Expand Down Expand Up @@ -71,12 +72,12 @@ type EC2Interface interface {
DetachSecurityGroupsFromNetworkInterface(groups []string, interfaceID string) error

DiscoverLaunchTemplateAMI(scope scope.LaunchTemplateScope) (*string, error)
GetLaunchTemplate(id string) (lt *expinfrav1.AWSLaunchTemplate, userDataHash string, userDataSecretKey *apimachinerytypes.NamespacedName, err error)
GetLaunchTemplate(id string) (lt *expinfrav1.AWSLaunchTemplate, userDataHash string, userDataSecretKey *apimachinerytypes.NamespacedName, bootstrapDataHash *string, err error)
GetLaunchTemplateID(id string) (string, error)
GetLaunchTemplateLatestVersion(id string) (string, error)
CreateLaunchTemplate(scope scope.LaunchTemplateScope, imageID *string, userDataSecretKey apimachinerytypes.NamespacedName, userData []byte) (string, error)
CreateLaunchTemplateVersion(id string, scope scope.LaunchTemplateScope, imageID *string, userDataSecretKey apimachinerytypes.NamespacedName, userData []byte) error
PruneLaunchTemplateVersions(id string) error
CreateLaunchTemplate(scope scope.LaunchTemplateScope, imageID *string, userDataSecretKey apimachinerytypes.NamespacedName, userData []byte, bootstrapDataHash string) (string, error)
CreateLaunchTemplateVersion(id string, scope scope.LaunchTemplateScope, imageID *string, userDataSecretKey apimachinerytypes.NamespacedName, userData []byte, bootstrapDataHash string) error
PruneLaunchTemplateVersions(id string) (*ec2.LaunchTemplateVersion, error)
DeleteLaunchTemplate(id string) error
LaunchTemplateNeedsUpdate(scope scope.LaunchTemplateScope, incoming *expinfrav1.AWSLaunchTemplate, existing *expinfrav1.AWSLaunchTemplate) (bool, error)
DeleteBastion() error
Expand Down Expand Up @@ -133,4 +134,5 @@ type ObjectStoreInterface interface {
Delete(m *scope.MachineScope) error
Create(m *scope.MachineScope, data []byte) (objectURL string, err error)
CreateForMachinePool(scope scope.LaunchTemplateScope, data []byte) (objectURL string, err error)
DeleteForMachinePool(scope scope.LaunchTemplateScope, bootstrapDataHash string) error
}
31 changes: 17 additions & 14 deletions pkg/cloud/services/mock_services/ec2_interface_mock.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions pkg/cloud/services/mock_services/objectstore_machine_interface_mock.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading