diff --git a/pkg/actions/cluster/cluster.go b/pkg/actions/cluster/cluster.go index a14fd7f0ae..b3d9825860 100644 --- a/pkg/actions/cluster/cluster.go +++ b/pkg/actions/cluster/cluster.go @@ -20,7 +20,7 @@ type Cluster interface { func New(cfg *api.ClusterConfig, ctl *eks.ClusterProvider) (Cluster, error) { clusterExists := true - err := ctl.RefreshClusterStatus(cfg) + err := ctl.RefreshClusterStatusIfStale(cfg) if err != nil { if awsError, ok := errors.Unwrap(errors.Unwrap(err)).(awserr.Error); ok && awsError.Code() == awseks.ErrCodeResourceNotFoundException { @@ -31,14 +31,14 @@ func New(cfg *api.ClusterConfig, ctl *eks.ClusterProvider) (Cluster, error) { } stackManager := ctl.NewStackManager(cfg) - hasClusterStack, err := stackManager.HasClusterStack() + clusterStack, err := stackManager.GetClusterStackIfExists() if err != nil { return nil, err } - if hasClusterStack { + if clusterStack != nil { logger.Debug("cluster %q was created by eksctl", cfg.Metadata.Name) - return NewOwnedCluster(cfg, ctl, stackManager), nil + return NewOwnedCluster(cfg, ctl, clusterStack, stackManager), nil } if !clusterExists { diff --git a/pkg/actions/cluster/owned.go b/pkg/actions/cluster/owned.go index 1d13de26e9..41ea0c57d8 100644 --- a/pkg/actions/cluster/owned.go +++ b/pkg/actions/cluster/owned.go @@ -18,14 +18,16 @@ import ( type OwnedCluster struct { cfg *api.ClusterConfig ctl *eks.ClusterProvider + clusterStack *manager.Stack stackManager manager.StackManager newClientSet func() (kubernetes.Interface, error) } -func NewOwnedCluster(cfg *api.ClusterConfig, ctl *eks.ClusterProvider, stackManager manager.StackManager) *OwnedCluster { +func NewOwnedCluster(cfg *api.ClusterConfig, ctl *eks.ClusterProvider, clusterStack *manager.Stack, stackManager manager.StackManager) *OwnedCluster { return &OwnedCluster{ cfg: cfg, ctl: ctl, + clusterStack: clusterStack, stackManager: stackManager, newClientSet: func() (kubernetes.Interface, error) { return ctl.NewStdClientSet(cfg) @@ -34,7 +36,7 @@ func NewOwnedCluster(cfg *api.ClusterConfig, ctl *eks.ClusterProvider, stackMana } func (c *OwnedCluster) Upgrade(dryRun bool) error { - if err := c.ctl.LoadClusterVPC(c.cfg, c.stackManager); err != nil { + if err := vpc.UseFromClusterStack(c.ctl.Provider, c.clusterStack, c.cfg); err != nil { return errors.Wrapf(err, "getting VPC configuration for cluster %q", c.cfg.Metadata.Name) } diff --git a/pkg/actions/cluster/owned_test.go b/pkg/actions/cluster/owned_test.go index 9d0cfe3fdf..ccbef90259 100644 --- a/pkg/actions/cluster/owned_test.go +++ b/pkg/actions/cluster/owned_test.go @@ -91,7 +91,7 @@ var _ = Describe("Delete", func() { }}}, }, nil) - c := cluster.NewOwnedCluster(cfg, ctl, fakeStackManager) + c := cluster.NewOwnedCluster(cfg, ctl, nil, fakeStackManager) fakeClientSet = fake.NewSimpleClientset() c.SetNewClientSet(func() (kubernetes.Interface, error) { @@ -135,7 +135,7 @@ var _ = Describe("Delete", func() { }}}, }, nil) - c := cluster.NewOwnedCluster(cfg, ctl, fakeStackManager) + c := cluster.NewOwnedCluster(cfg, ctl, nil, fakeStackManager) err := c.Delete(time.Microsecond, false, false) Expect(err).NotTo(HaveOccurred()) diff --git a/pkg/actions/fargate/create_test.go b/pkg/actions/fargate/create_test.go index 45c8e9ea7c..14a61ed31a 100644 --- a/pkg/actions/fargate/create_test.go +++ b/pkg/actions/fargate/create_test.go @@ -63,10 +63,6 @@ var _ = Describe("Fargate", func() { }) Context("owned cluster", func() { - BeforeEach(func() { - fakeStackManager.HasClusterStackReturns(true, nil) - }) - When("creating a farage role without specifying a role", func() { When("the fargate role doesn't exist", func() { BeforeEach(func() { diff --git a/pkg/cfn/manager/api.go b/pkg/cfn/manager/api.go index ae177e89eb..14a40f8ba4 100644 --- a/pkg/cfn/manager/api.go +++ b/pkg/cfn/manager/api.go @@ -555,30 +555,35 @@ func (c *StackCollection) DescribeStacks() ([]*Stack, error) { return stacks, nil } -func (c *StackCollection) HasClusterStack() (bool, error) { +func (c *StackCollection) GetClusterStackIfExists() (*Stack, error) { clusterStackNames, err := c.ListClusterStackNames() if err != nil { - return false, err + return nil, err } - return c.HasClusterStackUsingCachedList(clusterStackNames) + return c.getClusterStackUsingCachedList(clusterStackNames) } func (c *StackCollection) HasClusterStackUsingCachedList(clusterStackNames []string) (bool, error) { + stack, err := c.getClusterStackUsingCachedList(clusterStackNames) + return stack != nil, err +} + +func (c *StackCollection) getClusterStackUsingCachedList(clusterStackNames []string) (*Stack, error) { clusterStackName := c.MakeClusterStackName() for _, stack := range clusterStackNames { if stack == clusterStackName { stack, err := c.DescribeStack(&cloudformation.Stack{StackName: &clusterStackName}) if err != nil { - return false, err + return nil, err } for _, tag := range stack.Tags { if matchesClusterName(*tag.Key, *tag.Value, c.spec.Metadata.Name) { - return true, nil + return stack, nil } } } } - return false, nil + return nil, nil } // DescribeStackEvents describes the events that have occurred on the stack diff --git a/pkg/cfn/manager/fakes/fake_stack_manager.go b/pkg/cfn/manager/fakes/fake_stack_manager.go index 4a5b102d9c..7a82774855 100644 --- a/pkg/cfn/manager/fakes/fake_stack_manager.go +++ b/pkg/cfn/manager/fakes/fake_stack_manager.go @@ -280,6 +280,18 @@ type FakeStackManager struct { result1 string result2 error } + GetClusterStackIfExistsStub func() (*cloudformation.Stack, error) + getClusterStackIfExistsMutex sync.RWMutex + getClusterStackIfExistsArgsForCall []struct { + } + getClusterStackIfExistsReturns struct { + result1 *cloudformation.Stack + result2 error + } + getClusterStackIfExistsReturnsOnCall map[int]struct { + result1 *cloudformation.Stack + result2 error + } GetFargateStackStub func() (*cloudformation.Stack, error) getFargateStackMutex sync.RWMutex getFargateStackArgsForCall []struct { @@ -390,18 +402,6 @@ type FakeStackManager struct { result1 []*manager.NodeGroupSummary result2 error } - HasClusterStackStub func() (bool, error) - hasClusterStackMutex sync.RWMutex - hasClusterStackArgsForCall []struct { - } - hasClusterStackReturns struct { - result1 bool - result2 error - } - hasClusterStackReturnsOnCall map[int]struct { - result1 bool - result2 error - } HasClusterStackUsingCachedListStub func([]string) (bool, error) hasClusterStackUsingCachedListMutex sync.RWMutex hasClusterStackUsingCachedListArgsForCall []struct { @@ -1997,6 +1997,62 @@ func (fake *FakeStackManager) GetAutoScalingGroupNameReturnsOnCall(i int, result }{result1, result2} } +func (fake *FakeStackManager) GetClusterStackIfExists() (*cloudformation.Stack, error) { + fake.getClusterStackIfExistsMutex.Lock() + ret, specificReturn := fake.getClusterStackIfExistsReturnsOnCall[len(fake.getClusterStackIfExistsArgsForCall)] + fake.getClusterStackIfExistsArgsForCall = append(fake.getClusterStackIfExistsArgsForCall, struct { + }{}) + stub := fake.GetClusterStackIfExistsStub + fakeReturns := fake.getClusterStackIfExistsReturns + fake.recordInvocation("GetClusterStackIfExists", []interface{}{}) + fake.getClusterStackIfExistsMutex.Unlock() + if stub != nil { + return stub() + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *FakeStackManager) GetClusterStackIfExistsCallCount() int { + fake.getClusterStackIfExistsMutex.RLock() + defer fake.getClusterStackIfExistsMutex.RUnlock() + return len(fake.getClusterStackIfExistsArgsForCall) +} + +func (fake *FakeStackManager) GetClusterStackIfExistsCalls(stub func() (*cloudformation.Stack, error)) { + fake.getClusterStackIfExistsMutex.Lock() + defer fake.getClusterStackIfExistsMutex.Unlock() + fake.GetClusterStackIfExistsStub = stub +} + +func (fake *FakeStackManager) GetClusterStackIfExistsReturns(result1 *cloudformation.Stack, result2 error) { + fake.getClusterStackIfExistsMutex.Lock() + defer fake.getClusterStackIfExistsMutex.Unlock() + fake.GetClusterStackIfExistsStub = nil + fake.getClusterStackIfExistsReturns = struct { + result1 *cloudformation.Stack + result2 error + }{result1, result2} +} + +func (fake *FakeStackManager) GetClusterStackIfExistsReturnsOnCall(i int, result1 *cloudformation.Stack, result2 error) { + fake.getClusterStackIfExistsMutex.Lock() + defer fake.getClusterStackIfExistsMutex.Unlock() + fake.GetClusterStackIfExistsStub = nil + if fake.getClusterStackIfExistsReturnsOnCall == nil { + fake.getClusterStackIfExistsReturnsOnCall = make(map[int]struct { + result1 *cloudformation.Stack + result2 error + }) + } + fake.getClusterStackIfExistsReturnsOnCall[i] = struct { + result1 *cloudformation.Stack + result2 error + }{result1, result2} +} + func (fake *FakeStackManager) GetFargateStack() (*cloudformation.Stack, error) { fake.getFargateStackMutex.Lock() ret, specificReturn := fake.getFargateStackReturnsOnCall[len(fake.getFargateStackArgsForCall)] @@ -2543,62 +2599,6 @@ func (fake *FakeStackManager) GetUnmanagedNodeGroupSummariesReturnsOnCall(i int, }{result1, result2} } -func (fake *FakeStackManager) HasClusterStack() (bool, error) { - fake.hasClusterStackMutex.Lock() - ret, specificReturn := fake.hasClusterStackReturnsOnCall[len(fake.hasClusterStackArgsForCall)] - fake.hasClusterStackArgsForCall = append(fake.hasClusterStackArgsForCall, struct { - }{}) - stub := fake.HasClusterStackStub - fakeReturns := fake.hasClusterStackReturns - fake.recordInvocation("HasClusterStack", []interface{}{}) - fake.hasClusterStackMutex.Unlock() - if stub != nil { - return stub() - } - if specificReturn { - return ret.result1, ret.result2 - } - return fakeReturns.result1, fakeReturns.result2 -} - -func (fake *FakeStackManager) HasClusterStackCallCount() int { - fake.hasClusterStackMutex.RLock() - defer fake.hasClusterStackMutex.RUnlock() - return len(fake.hasClusterStackArgsForCall) -} - -func (fake *FakeStackManager) HasClusterStackCalls(stub func() (bool, error)) { - fake.hasClusterStackMutex.Lock() - defer fake.hasClusterStackMutex.Unlock() - fake.HasClusterStackStub = stub -} - -func (fake *FakeStackManager) HasClusterStackReturns(result1 bool, result2 error) { - fake.hasClusterStackMutex.Lock() - defer fake.hasClusterStackMutex.Unlock() - fake.HasClusterStackStub = nil - fake.hasClusterStackReturns = struct { - result1 bool - result2 error - }{result1, result2} -} - -func (fake *FakeStackManager) HasClusterStackReturnsOnCall(i int, result1 bool, result2 error) { - fake.hasClusterStackMutex.Lock() - defer fake.hasClusterStackMutex.Unlock() - fake.HasClusterStackStub = nil - if fake.hasClusterStackReturnsOnCall == nil { - fake.hasClusterStackReturnsOnCall = make(map[int]struct { - result1 bool - result2 error - }) - } - fake.hasClusterStackReturnsOnCall[i] = struct { - result1 bool - result2 error - }{result1, result2} -} - func (fake *FakeStackManager) HasClusterStackUsingCachedList(arg1 []string) (bool, error) { var arg1Copy []string if arg1 != nil { @@ -4219,6 +4219,8 @@ func (fake *FakeStackManager) Invocations() map[string][][]interface{} { defer fake.fixClusterCompatibilityMutex.RUnlock() fake.getAutoScalingGroupNameMutex.RLock() defer fake.getAutoScalingGroupNameMutex.RUnlock() + fake.getClusterStackIfExistsMutex.RLock() + defer fake.getClusterStackIfExistsMutex.RUnlock() fake.getFargateStackMutex.RLock() defer fake.getFargateStackMutex.RUnlock() fake.getIAMAddonNameMutex.RLock() @@ -4237,8 +4239,6 @@ func (fake *FakeStackManager) Invocations() map[string][][]interface{} { defer fake.getStackTemplateMutex.RUnlock() fake.getUnmanagedNodeGroupSummariesMutex.RLock() defer fake.getUnmanagedNodeGroupSummariesMutex.RUnlock() - fake.hasClusterStackMutex.RLock() - defer fake.hasClusterStackMutex.RUnlock() fake.hasClusterStackUsingCachedListMutex.RLock() defer fake.hasClusterStackUsingCachedListMutex.RUnlock() fake.listClusterStackNamesMutex.RLock() diff --git a/pkg/cfn/manager/interface.go b/pkg/cfn/manager/interface.go index 4bdf4b9c17..ed6485f0e8 100644 --- a/pkg/cfn/manager/interface.go +++ b/pkg/cfn/manager/interface.go @@ -39,7 +39,7 @@ type StackManager interface { DeleteStackBySpec(s *Stack) (*Stack, error) DeleteStackBySpecSync(s *Stack, errs chan error) error DescribeStacks() ([]*Stack, error) - HasClusterStack() (bool, error) + GetClusterStackIfExists() (*Stack, error) HasClusterStackUsingCachedList(clusterStackNames []string) (bool, error) DescribeStackEvents(i *Stack) ([]*cloudformation.StackEvent, error) LookupCloudTrailEvents(i *Stack) ([]*cloudtrail.Event, error) diff --git a/pkg/eks/eks.go b/pkg/eks/eks.go index 80af3471c3..14a5d0abd7 100644 --- a/pkg/eks/eks.go +++ b/pkg/eks/eks.go @@ -81,7 +81,7 @@ func (c *ClusterProvider) RefreshClusterStatus(spec *api.ClusterConfig) error { // SupportsManagedNodes reports whether an existing cluster supports Managed Nodes // The minimum required control plane version and platform version are 1.14 and eks.3 respectively func (c *ClusterProvider) SupportsManagedNodes(clusterConfig *api.ClusterConfig) (bool, error) { - if err := c.maybeRefreshClusterStatus(clusterConfig); err != nil { + if err := c.RefreshClusterStatusIfStale(clusterConfig); err != nil { return false, err } @@ -126,7 +126,7 @@ func ClusterSupportsManagedNodes(cluster *awseks.Cluster) (bool, error) { // SupportsFargate reports whether an existing cluster supports Fargate. func (c *ClusterProvider) SupportsFargate(clusterConfig *api.ClusterConfig) (bool, error) { - if err := c.maybeRefreshClusterStatus(clusterConfig); err != nil { + if err := c.RefreshClusterStatusIfStale(clusterConfig); err != nil { return false, err } return ClusterSupportsFargate(c.Status.ClusterInfo.Cluster) @@ -179,7 +179,8 @@ func PlatformVersion(platformVersion string) (int, error) { return version, nil } -func (c *ClusterProvider) maybeRefreshClusterStatus(spec *api.ClusterConfig) error { +// RefreshClusterStatusIfStale refreshes the cluster status if enough time has passed since the last refresh +func (c *ClusterProvider) RefreshClusterStatusIfStale(spec *api.ClusterConfig) error { if c.clusterInfoNeedsUpdate() { return c.RefreshClusterStatus(spec) } @@ -188,7 +189,7 @@ func (c *ClusterProvider) maybeRefreshClusterStatus(spec *api.ClusterConfig) err // CanDelete return true when a cluster can be deleted, otherwise it returns false along with an error explaining the reason func (c *ClusterProvider) CanDelete(spec *api.ClusterConfig) (bool, error) { - err := c.maybeRefreshClusterStatus(spec) + err := c.RefreshClusterStatusIfStale(spec) if err != nil { if awsError, ok := errors.Unwrap(errors.Unwrap(err)).(awserr.Error); ok && awsError.Code() == awseks.ErrCodeResourceNotFoundException { @@ -202,7 +203,7 @@ func (c *ClusterProvider) CanDelete(spec *api.ClusterConfig) (bool, error) { // CanOperate returns true when a cluster can be operated, otherwise it returns false along with an error explaining the reason func (c *ClusterProvider) CanOperate(spec *api.ClusterConfig) (bool, error) { - err := c.maybeRefreshClusterStatus(spec) + err := c.RefreshClusterStatusIfStale(spec) if err != nil { return false, errors.Wrapf(err, "unable to fetch cluster status to determine operability") } @@ -217,7 +218,7 @@ func (c *ClusterProvider) CanOperate(spec *api.ClusterConfig) (bool, error) { // CanUpdate return true when a cluster or add-ons can be updated, otherwise it returns false along with an error explaining the reason func (c *ClusterProvider) CanUpdate(spec *api.ClusterConfig) (bool, error) { - err := c.maybeRefreshClusterStatus(spec) + err := c.RefreshClusterStatusIfStale(spec) if err != nil { return false, errors.Wrapf(err, "fetching cluster status to determine update status") }