Skip to content

Commit

Permalink
fix: EKS node lifecycle discovery configuration value nil handling
Browse files Browse the repository at this point in the history
  • Loading branch information
Laimonas Rastenis committed Oct 23, 2023
1 parent 8abd2de commit 38ee920
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 2 deletions.
4 changes: 3 additions & 1 deletion internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ type Mode string
const (
ModeAgent Mode = "agent"
ModeMonitor Mode = "monitor"

DefaultAPINodeLifecycleDiscoveryEnabled = true
)

type TLS struct {
Expand Down Expand Up @@ -196,7 +198,7 @@ func Get() Config {
cfg.EKS.APITimeout = 120 * time.Second
}
if cfg.EKS.APINodeLifecycleDiscoveryEnabled == nil {
cfg.EKS.APINodeLifecycleDiscoveryEnabled = lo.ToPtr(true)
cfg.EKS.APINodeLifecycleDiscoveryEnabled = lo.ToPtr(DefaultAPINodeLifecycleDiscoveryEnabled)
}
}

Expand Down
11 changes: 10 additions & 1 deletion internal/services/providers/providers.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ func GetProvider(ctx context.Context, log logrus.FieldLogger, discoveryService d

if cfg.Provider == eks.Name || cfg.EKS != nil {
eksProviderLogger := log.WithField("provider", eks.Name)
apiNodeLifecycleDiscoveryEnabled := *cfg.EKS.APINodeLifecycleDiscoveryEnabled
apiNodeLifecycleDiscoveryEnabled := isAPINodeLifecycleDiscoveryEnabled(cfg)

if !apiNodeLifecycleDiscoveryEnabled {
eksProviderLogger.Info("node lifecycle discovery through AWS API is disabled - all nodes without spot labels will be considered on-demand")
}
Expand All @@ -48,3 +49,11 @@ func GetProvider(ctx context.Context, log logrus.FieldLogger, discoveryService d

return nil, fmt.Errorf("unknown provider %q", cfg.Provider)
}

func isAPINodeLifecycleDiscoveryEnabled(cfg config.Config) bool {
if cfg.EKS != nil && cfg.EKS.APINodeLifecycleDiscoveryEnabled != nil {
return *cfg.EKS.APINodeLifecycleDiscoveryEnabled
}

return config.DefaultAPINodeLifecycleDiscoveryEnabled
}
37 changes: 37 additions & 0 deletions internal/services/providers/providers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"os"
"testing"

"github.com/samber/lo"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/require"

Expand Down Expand Up @@ -86,3 +87,39 @@ func TestGetProvider(t *testing.T) {
r.IsType(&openshift.Provider{}, got)
})
}

func Test_isAPINodeLifecycleDiscoveryEnabled(t *testing.T) {
tests := map[string]struct {
cfg config.Config
want bool
}{
"should use default node lifecycle discovery value when EKS config is nil": {
cfg: config.Config{},
want: true,
},
"should use default node lifecycle discovery value when EKS config is not nil and config value is nil": {
cfg: config.Config{
EKS: &config.EKS{APINodeLifecycleDiscoveryEnabled: nil},
},
want: true,
},
"should use node lifecycle discovery value from config when it is configured": {
cfg: config.Config{
EKS: &config.EKS{
APINodeLifecycleDiscoveryEnabled: lo.ToPtr(false),
},
},
want: false,
},
}

for testName, tt := range tests {
t.Run(testName, func(t *testing.T) {
r := require.New(t)

got := isAPINodeLifecycleDiscoveryEnabled(tt.cfg)

r.Equal(tt.want, got)
})
}
}

0 comments on commit 38ee920

Please sign in to comment.