From 57190cdd67e7b6eb9909f41d4ec80eefdc978c19 Mon Sep 17 00:00:00 2001 From: Cameron Meissner Date: Mon, 25 Mar 2024 14:15:35 -0700 Subject: [PATCH 1/7] feat: toggles in agentbakersvc --- apiserver/apiserver.go | 5 +- apiserver/getdistrosigimageconfig.go | 8 +- apiserver/getlatestsigimageconfig.go | 8 +- apiserver/getnodebootstrapdata.go | 2 +- cmd/main.go | 3 +- cmd/starter/start.go | 9 +- fuzz/api/main.go | 3 +- pkg/agent/baker_test.go | 9 +- pkg/agent/bakerapi.go | 100 +++++--- pkg/agent/bakerapi_test.go | 277 ++++++++++++++++++++- pkg/agent/datamodel/sig_config.go | 15 ++ pkg/agent/datamodel/types.go | 8 +- pkg/agent/toggles/fieldnames/fieldnames.go | 7 + pkg/agent/toggles/toggles.go | 6 + pkg/agent/toggles/toggles_suite_test.go | 13 + pkg/agent/toggles/toggles_test.go | 82 ++++++ pkg/agent/toggles/types.go | 74 ++++++ 17 files changed, 571 insertions(+), 58 deletions(-) create mode 100644 pkg/agent/toggles/fieldnames/fieldnames.go create mode 100644 pkg/agent/toggles/toggles.go create mode 100644 pkg/agent/toggles/toggles_suite_test.go create mode 100644 pkg/agent/toggles/toggles_test.go create mode 100644 pkg/agent/toggles/types.go diff --git a/apiserver/apiserver.go b/apiserver/apiserver.go index ec77a5b5afb..9518c186abc 100644 --- a/apiserver/apiserver.go +++ b/apiserver/apiserver.go @@ -6,6 +6,8 @@ import ( "log" "net/http" "time" + + "github.com/Azure/agentbaker/pkg/agent/toggles" ) const ( @@ -14,7 +16,8 @@ const ( // Options holds the options for the api server. type Options struct { - Addr string + Addr string + Toggles *toggles.Toggles } func (o *Options) validate() error { diff --git a/apiserver/getdistrosigimageconfig.go b/apiserver/getdistrosigimageconfig.go index 987a730e74b..8794288e0c2 100644 --- a/apiserver/getdistrosigimageconfig.go +++ b/apiserver/getdistrosigimageconfig.go @@ -26,14 +26,18 @@ func (api *APIServer) GetDistroSigImageConfig(w http.ResponseWriter, r *http.Req return } - agentBaker, err := agent.NewAgentBaker() + agentBaker, err := agent.NewAgentBaker(api.Options.Toggles) if err != nil { log.Println(err.Error()) http.Error(w, err.Error(), http.StatusBadRequest) return } - allDistros, err := agentBaker.GetDistroSigImageConfig(config.SIGConfig, config.Region) + allDistros, err := agentBaker.GetDistroSigImageConfig(config.SIGConfig, &datamodel.EnvironmentConfig{ + SubscriptionID: config.SubscriptionID, + TenantID: config.TenantID, + Region: config.Region, + }) if err != nil { log.Println(err.Error()) http.Error(w, err.Error(), http.StatusBadRequest) diff --git a/apiserver/getlatestsigimageconfig.go b/apiserver/getlatestsigimageconfig.go index 2b25af109e7..326a791b2ad 100644 --- a/apiserver/getlatestsigimageconfig.go +++ b/apiserver/getlatestsigimageconfig.go @@ -26,14 +26,18 @@ func (api *APIServer) GetLatestSigImageConfig(w http.ResponseWriter, r *http.Req return } - agentBaker, err := agent.NewAgentBaker() + agentBaker, err := agent.NewAgentBaker(api.Options.Toggles) if err != nil { log.Println(err.Error()) http.Error(w, err.Error(), http.StatusBadRequest) return } - latestSigConfig, err := agentBaker.GetLatestSigImageConfig(config.SIGConfig, config.Region, config.Distro) + latestSigConfig, err := agentBaker.GetLatestSigImageConfig(config.SIGConfig, config.Distro, &datamodel.EnvironmentConfig{ + SubscriptionID: config.SubscriptionID, + TenantID: config.TenantID, + Region: config.Region, + }) if err != nil { log.Println(err.Error()) http.Error(w, err.Error(), http.StatusBadRequest) diff --git a/apiserver/getnodebootstrapdata.go b/apiserver/getnodebootstrapdata.go index adbeb4d4390..421a7ad2e1f 100644 --- a/apiserver/getnodebootstrapdata.go +++ b/apiserver/getnodebootstrapdata.go @@ -33,7 +33,7 @@ func (api *APIServer) GetNodeBootstrapData(w http.ResponseWriter, r *http.Reques return } - agentBaker, err := agent.NewAgentBaker() + agentBaker, err := agent.NewAgentBaker(api.Options.Toggles) if err != nil { log.Println(err.Error()) http.Error(w, err.Error(), http.StatusBadRequest) diff --git a/cmd/main.go b/cmd/main.go index 64df3e1cb6b..19aaff8fb33 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -3,10 +3,11 @@ package main import ( "github.com/Azure/agentbaker/cmd/starter" + "github.com/Azure/agentbaker/pkg/agent/toggles" ) // NOTE: please make the main.go only contains the starter.Execute(). // that's because other components is reference the same function "starter.Execute()" to start the process. func main() { - starter.Execute() + starter.Execute(toggles.NewToggles()) } diff --git a/cmd/starter/start.go b/cmd/starter/start.go index 957af61dacc..2761d9380b1 100644 --- a/cmd/starter/start.go +++ b/cmd/starter/start.go @@ -9,11 +9,18 @@ import ( "syscall" "github.com/Azure/agentbaker/apiserver" + agenttoggles "github.com/Azure/agentbaker/pkg/agent/toggles" "github.com/spf13/cobra" ) // Execute adds all child commands to the root command and sets flags appropriately. -func Execute() { +func Execute(toggles *agenttoggles.Toggles) { + // set toggles + options.Toggles = agenttoggles.NewToggles() + if toggles != nil { + options.Toggles = toggles + } + rootCmd.AddCommand(startCmd) startCmd.Flags().StringVar(&options.Addr, "addr", ":8080", "the addr to serve the api on") diff --git a/fuzz/api/main.go b/fuzz/api/main.go index 326ad905715..af2a221bcd9 100644 --- a/fuzz/api/main.go +++ b/fuzz/api/main.go @@ -6,6 +6,7 @@ import ( "github.com/Azure/agentbaker/pkg/agent" "github.com/Azure/agentbaker/pkg/agent/datamodel" + "github.com/Azure/agentbaker/pkg/agent/toggles" ) func Fuzz(data []byte) int { @@ -15,7 +16,7 @@ func Fuzz(data []byte) int { return -1 } - baker, err := agent.NewAgentBaker() + baker, err := agent.NewAgentBaker(toggles.NewToggles()) if err != nil { return -1 } diff --git a/pkg/agent/baker_test.go b/pkg/agent/baker_test.go index 5734109fe9f..cd1d8af84f5 100644 --- a/pkg/agent/baker_test.go +++ b/pkg/agent/baker_test.go @@ -17,6 +17,7 @@ import ( "strings" "github.com/Azure/agentbaker/pkg/agent/datamodel" + "github.com/Azure/agentbaker/pkg/agent/toggles" "github.com/Azure/go-autorest/autorest/to" "github.com/barkimedes/go-deepcopy" . "github.com/onsi/ginkgo" @@ -220,7 +221,7 @@ var _ = Describe("Assert generated customData and cseCmd", func() { Expect(err).To(BeNil()) // customData - ab, err := NewAgentBaker() + ab, err := NewAgentBaker(toggles.NewToggles()) Expect(err).To(BeNil()) nodeBootstrapping, err := ab.GetNodeBootstrapping( context.Background(), @@ -240,7 +241,7 @@ var _ = Describe("Assert generated customData and cseCmd", func() { Expect(customData).To(Equal(string(expectedCustomData))) // CSE - ab, err = NewAgentBaker() + ab, err = NewAgentBaker(toggles.NewToggles()) Expect(err).To(BeNil()) nodeBootstrapping, err = ab.GetNodeBootstrapping( context.Background(), @@ -1454,7 +1455,7 @@ var _ = Describe("Assert generated customData and cseCmd for Windows", func() { } // customData - ab, err := NewAgentBaker() + ab, err := NewAgentBaker(toggles.NewToggles()) Expect(err).To(BeNil()) nodeBootstrapping, err := ab.GetNodeBootstrapping(context.Background(), config) Expect(err).To(BeNil()) @@ -1474,7 +1475,7 @@ var _ = Describe("Assert generated customData and cseCmd for Windows", func() { Expect(customData).To(Equal(string(expectedCustomData))) // CSE - ab, err = NewAgentBaker() + ab, err = NewAgentBaker(toggles.NewToggles()) Expect(err).To(BeNil()) nodeBootstrapping, err = ab.GetNodeBootstrapping(context.Background(), config) Expect(err).To(BeNil()) diff --git a/pkg/agent/bakerapi.go b/pkg/agent/bakerapi.go index 993c6c6090f..5b30da6165d 100644 --- a/pkg/agent/bakerapi.go +++ b/pkg/agent/bakerapi.go @@ -8,24 +8,28 @@ import ( "fmt" "github.com/Azure/agentbaker/pkg/agent/datamodel" + "github.com/Azure/agentbaker/pkg/agent/toggles" ) //nolint:revive // Name does not need to be modified to baker type AgentBaker interface { GetNodeBootstrapping(ctx context.Context, config *datamodel.NodeBootstrappingConfiguration) (*datamodel.NodeBootstrapping, error) - GetLatestSigImageConfig(sigConfig datamodel.SIGConfig, region string, distro datamodel.Distro) (*datamodel.SigImageConfig, error) - GetDistroSigImageConfig(sigConfig datamodel.SIGConfig, region string) (map[datamodel.Distro]datamodel.SigImageConfig, error) + GetLatestSigImageConfig(sigConfig datamodel.SIGConfig, distro datamodel.Distro, envConfig *datamodel.EnvironmentConfig) (*datamodel.SigImageConfig, error) + GetDistroSigImageConfig(sigConfig datamodel.SIGConfig, envConfig *datamodel.EnvironmentConfig) (map[datamodel.Distro]datamodel.SigImageConfig, error) } -func NewAgentBaker() (AgentBaker, error) { - return &agentBakerImpl{}, nil +func NewAgentBaker(toggles *toggles.Toggles) (AgentBaker, error) { + return &agentBakerImpl{ + toggles: toggles, + }, nil } -type agentBakerImpl struct{} +type agentBakerImpl struct { + toggles *toggles.Toggles +} //nolint:revive, nolintlint // ctx is not used, but may be in the future -func (agentBaker *agentBakerImpl) GetNodeBootstrapping(ctx context.Context, - config *datamodel.NodeBootstrappingConfiguration) (*datamodel.NodeBootstrapping, error) { +func (agentBaker *agentBakerImpl) GetNodeBootstrapping(ctx context.Context, config *datamodel.NodeBootstrappingConfiguration) (*datamodel.NodeBootstrapping, error) { // validate and fix input before passing config to the template generator. if config.AgentPoolProfile.IsWindows() { validateAndSetWindowsNodeBootstrappingConfiguration(config) @@ -63,70 +67,102 @@ func (agentBaker *agentBakerImpl) GetNodeBootstrapping(ctx context.Context, return nil, fmt.Errorf("can't find image for distro %s", distro) } - return nodeBootstrapping, nil -} - -func findSIGImageConfig(sigConfig datamodel.SIGAzureEnvironmentSpecConfig, distro datamodel.Distro) *datamodel.SigImageConfig { - if imageConfig, ok := sigConfig.SigUbuntuImageConfig[distro]; ok { - return &imageConfig - } - if imageConfig, ok := sigConfig.SigCBLMarinerImageConfig[distro]; ok { - return &imageConfig - } - if imageConfig, ok := sigConfig.SigAzureLinuxImageConfig[distro]; ok { - return &imageConfig - } - if imageConfig, ok := sigConfig.SigWindowsImageConfig[distro]; ok { - return &imageConfig - } - if imageConfig, ok := sigConfig.SigUbuntuEdgeZoneImageConfig[distro]; ok { - return &imageConfig + if !config.AgentPoolProfile.IsWindows() { + // handle node image version toggle/override + e := toggles.NewEntityFromNodeBootstrappingConfiguration(config) + imageVersionOverrides := agentBaker.toggles.GetLinuxNodeImageVersion(e) + if imageVersion, ok := imageVersionOverrides[string(distro)]; ok { + nodeBootstrapping.SigImageConfig.Version = imageVersion + } } - return nil + return nodeBootstrapping, nil } -func (agentBaker *agentBakerImpl) GetLatestSigImageConfig( - sigConfig datamodel.SIGConfig, region string, distro datamodel.Distro) (*datamodel.SigImageConfig, error) { - sigAzureEnvironmentSpecConfig, err := datamodel.GetSIGAzureCloudSpecConfig(sigConfig, region) +func (agentBaker *agentBakerImpl) GetLatestSigImageConfig(sigConfig datamodel.SIGConfig, + distro datamodel.Distro, envConfig *datamodel.EnvironmentConfig) (*datamodel.SigImageConfig, error) { + sigAzureEnvironmentSpecConfig, err := datamodel.GetSIGAzureCloudSpecConfig(sigConfig, envConfig.Region) if err != nil { return nil, err } sigImageConfig := findSIGImageConfig(sigAzureEnvironmentSpecConfig, distro) if sigImageConfig == nil { - return nil, fmt.Errorf("can't find SIG image config for distro %s in region %s", distro, region) + return nil, fmt.Errorf("can't find SIG image config for distro %s in region %s", distro, envConfig.Region) + } + + if !distro.IsWindowsDistro() { + e := toggles.NewEntityFromEnvironmentConfig(envConfig) + imageVersionOverrides := agentBaker.toggles.GetLinuxNodeImageVersion(e) + if imageVersion, ok := imageVersionOverrides[string(distro)]; ok { + sigImageConfig.Version = imageVersion + } } return sigImageConfig, nil } func (agentBaker *agentBakerImpl) GetDistroSigImageConfig( - sigConfig datamodel.SIGConfig, region string) (map[datamodel.Distro]datamodel.SigImageConfig, error) { - allAzureSigConfig, err := datamodel.GetSIGAzureCloudSpecConfig(sigConfig, region) + sigConfig datamodel.SIGConfig, envConfig *datamodel.EnvironmentConfig) (map[datamodel.Distro]datamodel.SigImageConfig, error) { + allAzureSigConfig, err := datamodel.GetSIGAzureCloudSpecConfig(sigConfig, envConfig.Region) if err != nil { return nil, fmt.Errorf("failed to get sig image config: %w", err) } + e := toggles.NewEntityFromEnvironmentConfig(envConfig) + linuxImageVersionOverrides := agentBaker.toggles.GetLinuxNodeImageVersion(e) + allDistros := map[datamodel.Distro]datamodel.SigImageConfig{} for distro, sigConfig := range allAzureSigConfig.SigWindowsImageConfig { allDistros[distro] = sigConfig } for distro, sigConfig := range allAzureSigConfig.SigCBLMarinerImageConfig { + if version, ok := linuxImageVersionOverrides[string(distro)]; ok { + sigConfig.Version = version + } allDistros[distro] = sigConfig } for distro, sigConfig := range allAzureSigConfig.SigAzureLinuxImageConfig { + if version, ok := linuxImageVersionOverrides[string(distro)]; ok { + sigConfig.Version = version + } allDistros[distro] = sigConfig } for distro, sigConfig := range allAzureSigConfig.SigUbuntuImageConfig { + if version, ok := linuxImageVersionOverrides[string(distro)]; ok { + sigConfig.Version = version + } allDistros[distro] = sigConfig } for distro, sigConfig := range allAzureSigConfig.SigUbuntuEdgeZoneImageConfig { + if version, ok := linuxImageVersionOverrides[string(distro)]; ok { + sigConfig.Version = version + } allDistros[distro] = sigConfig } return allDistros, nil } + +func findSIGImageConfig(sigConfig datamodel.SIGAzureEnvironmentSpecConfig, distro datamodel.Distro) *datamodel.SigImageConfig { + if imageConfig, ok := sigConfig.SigUbuntuImageConfig[distro]; ok { + return &imageConfig + } + if imageConfig, ok := sigConfig.SigCBLMarinerImageConfig[distro]; ok { + return &imageConfig + } + if imageConfig, ok := sigConfig.SigAzureLinuxImageConfig[distro]; ok { + return &imageConfig + } + if imageConfig, ok := sigConfig.SigWindowsImageConfig[distro]; ok { + return &imageConfig + } + if imageConfig, ok := sigConfig.SigUbuntuEdgeZoneImageConfig[distro]; ok { + return &imageConfig + } + + return nil +} diff --git a/pkg/agent/bakerapi_test.go b/pkg/agent/bakerapi_test.go index 8b2fd649f42..9e4e539e855 100644 --- a/pkg/agent/bakerapi_test.go +++ b/pkg/agent/bakerapi_test.go @@ -4,6 +4,7 @@ import ( "context" "github.com/Azure/agentbaker/pkg/agent/datamodel" + agenttoggles "github.com/Azure/agentbaker/pkg/agent/toggles" "github.com/barkimedes/go-deepcopy" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -14,9 +15,12 @@ var _ = Describe("AgentBaker API implementation tests", func() { cs *datamodel.ContainerService config *datamodel.NodeBootstrappingConfiguration sigConfig *datamodel.SIGConfig + toggles *agenttoggles.Toggles ) BeforeEach(func() { + toggles = agenttoggles.NewToggles() + cs = &datamodel.ContainerService{ Location: "southcentralus", Type: "Microsoft.ContainerService/ManagedClusters", @@ -155,7 +159,7 @@ var _ = Describe("AgentBaker API implementation tests", func() { Context("GetNodeBootstrapping", func() { It("should return correct boot strapping data", func() { - agentBaker, err := NewAgentBaker() + agentBaker, err := NewAgentBaker(toggles) Expect(err).NotTo(HaveOccurred()) nodeBootStrapping, err := agentBaker.GetNodeBootstrapping(context.Background(), config) @@ -177,6 +181,57 @@ var _ = Describe("AgentBaker API implementation tests", func() { Expect(nodeBootStrapping.SigImageConfig.Version).To(Equal("2021.11.06")) }) + It("should return the correct bootstrapping data when linux node image version override is present", func() { + toggles.MapToggles = map[string]agenttoggles.MapToggle{ + "linux-node-image-version": func(entity *agenttoggles.Entity) map[string]string { + return map[string]string{ + string(datamodel.AKSUbuntu1604): "202402.27.0", + } + }, + } + + agentBaker, err := NewAgentBaker(toggles) + Expect(err).NotTo(HaveOccurred()) + + nodeBootStrapping, err := agentBaker.GetNodeBootstrapping(context.Background(), config) + Expect(err).NotTo(HaveOccurred()) + + // baker_test.go tested the correctness of the generated Custom Data and CSE, so here + // we just do a sanity check of them not being empty. + Expect(nodeBootStrapping.CustomData).NotTo(Equal("")) + Expect(nodeBootStrapping.CSE).NotTo(Equal("")) + + Expect(nodeBootStrapping.SigImageConfig.ResourceGroup).To(Equal("resourcegroup")) + Expect(nodeBootStrapping.SigImageConfig.Gallery).To(Equal("aksubuntu")) + Expect(nodeBootStrapping.SigImageConfig.Definition).To(Equal("1604")) + Expect(nodeBootStrapping.SigImageConfig.Version).To(Equal("202402.27.0")) + }) + + It("should return the correct bootstrapping data when linux node image version is present but does not specify for distro", func() { + toggles.MapToggles = map[string]agenttoggles.MapToggle{ + "linux-node-image-version": func(entity *agenttoggles.Entity) map[string]string { + return map[string]string{ + string(datamodel.AKSUbuntu1804): "202402.27.0", + } + }, + } + agentBaker, err := NewAgentBaker(toggles) + Expect(err).NotTo(HaveOccurred()) + + nodeBootStrapping, err := agentBaker.GetNodeBootstrapping(context.Background(), config) + Expect(err).NotTo(HaveOccurred()) + + // baker_test.go tested the correctness of the generated Custom Data and CSE, so here + // we just do a sanity check of them not being empty. + Expect(nodeBootStrapping.CustomData).NotTo(Equal("")) + Expect(nodeBootStrapping.CSE).NotTo(Equal("")) + + Expect(nodeBootStrapping.SigImageConfig.ResourceGroup).To(Equal("resourcegroup")) + Expect(nodeBootStrapping.SigImageConfig.Gallery).To(Equal("aksubuntu")) + Expect(nodeBootStrapping.SigImageConfig.Definition).To(Equal("1604")) + Expect(nodeBootStrapping.SigImageConfig.Version).To(Equal("2021.11.06")) + }) + It("should return an error if cloud is not found", func() { // this CloudSpecConfig is shared across all AgentBaker UTs, // thus we need to make and use a copy when performing mutations for mocking @@ -187,7 +242,7 @@ var _ = Describe("AgentBaker API implementation tests", func() { config.CloudSpecConfig = cloudSpecConfig config.CloudSpecConfig.CloudName = "UnknownCloud" - agentBaker, err := NewAgentBaker() + agentBaker, err := NewAgentBaker(toggles) Expect(err).NotTo(HaveOccurred()) _, err = agentBaker.GetNodeBootstrapping(context.Background(), config) Expect(err).To(HaveOccurred()) @@ -195,7 +250,7 @@ var _ = Describe("AgentBaker API implementation tests", func() { It("should return an error if distro is neither found in PIR nor found in SIG", func() { config.AgentPoolProfile.Distro = "unknown" - agentBaker, err := NewAgentBaker() + agentBaker, err := NewAgentBaker(toggles) Expect(err).NotTo(HaveOccurred()) _, err = agentBaker.GetNodeBootstrapping(context.Background(), config) @@ -204,7 +259,7 @@ var _ = Describe("AgentBaker API implementation tests", func() { It("should not return an error for customized image", func() { config.AgentPoolProfile.Distro = datamodel.CustomizedImage - agentBaker, err := NewAgentBaker() + agentBaker, err := NewAgentBaker(toggles) Expect(err).NotTo(HaveOccurred()) _, err = agentBaker.GetNodeBootstrapping(context.Background(), config) @@ -213,7 +268,7 @@ var _ = Describe("AgentBaker API implementation tests", func() { It("should not return an error for customized kata image", func() { config.AgentPoolProfile.Distro = datamodel.CustomizedImageKata - agentBaker, err := NewAgentBaker() + agentBaker, err := NewAgentBaker(toggles) Expect(err).NotTo(HaveOccurred()) _, err = agentBaker.GetNodeBootstrapping(context.Background(), config) @@ -222,7 +277,7 @@ var _ = Describe("AgentBaker API implementation tests", func() { It("should not return an error for customized windows image", func() { config.AgentPoolProfile.Distro = datamodel.CustomizedWindowsOSImage - agentBaker, err := NewAgentBaker() + agentBaker, err := NewAgentBaker(toggles) Expect(err).NotTo(HaveOccurred()) _, err = agentBaker.GetNodeBootstrapping(context.Background(), config) @@ -232,10 +287,62 @@ var _ = Describe("AgentBaker API implementation tests", func() { Context("GetLatestSigImageConfig", func() { It("should return correct value for existing distro", func() { - agentBaker, err := NewAgentBaker() + agentBaker, err := NewAgentBaker(toggles) Expect(err).NotTo(HaveOccurred()) - sigImageConfig, err := agentBaker.GetLatestSigImageConfig(config.SIGConfig, cs.Location, datamodel.AKSUbuntu1604) + sigImageConfig, err := agentBaker.GetLatestSigImageConfig(config.SIGConfig, datamodel.AKSUbuntu1604, &datamodel.EnvironmentConfig{ + SubscriptionID: config.SubscriptionID, + TenantID: config.TenantID, + Region: cs.Location, + }) + Expect(err).NotTo(HaveOccurred()) + + Expect(sigImageConfig.ResourceGroup).To(Equal("resourcegroup")) + Expect(sigImageConfig.Gallery).To(Equal("aksubuntu")) + Expect(sigImageConfig.Definition).To(Equal("1604")) + Expect(sigImageConfig.Version).To(Equal("2021.11.06")) + }) + + It("should return correct value for existing distro when linux node image version override is provided", func() { + toggles.MapToggles = map[string]agenttoggles.MapToggle{ + "linux-node-image-version": func(entity *agenttoggles.Entity) map[string]string { + return map[string]string{ + string(datamodel.AKSUbuntu1604): "202402.27.0", + } + }, + } + agentBaker, err := NewAgentBaker(toggles) + Expect(err).NotTo(HaveOccurred()) + + sigImageConfig, err := agentBaker.GetLatestSigImageConfig(config.SIGConfig, datamodel.AKSUbuntu1604, &datamodel.EnvironmentConfig{ + SubscriptionID: config.SubscriptionID, + TenantID: config.TenantID, + Region: cs.Location, + }) + Expect(err).NotTo(HaveOccurred()) + + Expect(sigImageConfig.ResourceGroup).To(Equal("resourcegroup")) + Expect(sigImageConfig.Gallery).To(Equal("aksubuntu")) + Expect(sigImageConfig.Definition).To(Equal("1604")) + Expect(sigImageConfig.Version).To(Equal("202402.27.0")) + }) + + It("should return correct value for existing distro when linux node image version override is provided but not for distro", func() { + toggles.MapToggles = map[string]agenttoggles.MapToggle{ + "linux-node-image-version": func(entity *agenttoggles.Entity) map[string]string { + return map[string]string{ + string(datamodel.AKSUbuntu1804): "202402.27.0", + } + }, + } + agentBaker, err := NewAgentBaker(toggles) + Expect(err).NotTo(HaveOccurred()) + + sigImageConfig, err := agentBaker.GetLatestSigImageConfig(config.SIGConfig, datamodel.AKSUbuntu1604, &datamodel.EnvironmentConfig{ + SubscriptionID: config.SubscriptionID, + TenantID: config.TenantID, + Region: cs.Location, + }) Expect(err).NotTo(HaveOccurred()) Expect(sigImageConfig.ResourceGroup).To(Equal("resourcegroup")) @@ -245,11 +352,161 @@ var _ = Describe("AgentBaker API implementation tests", func() { }) It("should return error if image config not found for distro", func() { - agentBaker, err := NewAgentBaker() + agentBaker, err := NewAgentBaker(toggles) Expect(err).NotTo(HaveOccurred()) - _, err = agentBaker.GetLatestSigImageConfig(config.SIGConfig, cs.Location, "unknown") + _, err = agentBaker.GetLatestSigImageConfig(config.SIGConfig, "unknown", &datamodel.EnvironmentConfig{ + SubscriptionID: config.SubscriptionID, + TenantID: config.TenantID, + Region: cs.Location, + }) Expect(err).To(HaveOccurred()) }) }) + + Context("GetDistroSigImageConfig", func() { + var ( + ubuntuDistros []datamodel.Distro + marinerDistros []datamodel.Distro + azureLinuxDistros []datamodel.Distro + allLinuxDistros []datamodel.Distro + ) + + BeforeEach(func() { + ubuntuDistros = []datamodel.Distro{ + datamodel.AKSUbuntuContainerd1804, + datamodel.AKSUbuntuContainerd1804Gen2, + datamodel.AKSUbuntuGPUContainerd1804, + datamodel.AKSUbuntuGPUContainerd1804Gen2, + datamodel.AKSUbuntuFipsContainerd1804, + datamodel.AKSUbuntuFipsContainerd1804Gen2, + datamodel.AKSUbuntuFipsContainerd2004, + datamodel.AKSUbuntuFipsContainerd2004Gen2, + datamodel.AKSUbuntuContainerd2204, + datamodel.AKSUbuntuContainerd2204Gen2, + datamodel.AKSUbuntuContainerd2004CVMGen2, + datamodel.AKSUbuntuArm64Containerd2204Gen2, + datamodel.AKSUbuntuContainerd2204TLGen2, + } + + marinerDistros = []datamodel.Distro{ + datamodel.AKSCBLMarinerV2, + datamodel.AKSCBLMarinerV2Gen2, + datamodel.AKSCBLMarinerV2FIPS, + datamodel.AKSCBLMarinerV2Gen2FIPS, + datamodel.AKSCBLMarinerV2Gen2Kata, + datamodel.AKSCBLMarinerV2Arm64Gen2, + datamodel.AKSCBLMarinerV2Gen2TL, + } + + azureLinuxDistros = []datamodel.Distro{ + datamodel.AKSAzureLinuxV2, + datamodel.AKSAzureLinuxV2Gen2, + datamodel.AKSAzureLinuxV2FIPS, + datamodel.AKSAzureLinuxV2Gen2FIPS, + datamodel.AKSAzureLinuxV2Gen2Kata, + datamodel.AKSAzureLinuxV2Arm64Gen2, + datamodel.AKSAzureLinuxV2Gen2TL, + } + + allLinuxDistros = append(allLinuxDistros, ubuntuDistros...) + allLinuxDistros = append(allLinuxDistros, marinerDistros...) + allLinuxDistros = append(allLinuxDistros, azureLinuxDistros...) + }) + + It("should return correct value for all existing distros", func() { + agentBaker, err := NewAgentBaker(toggles) + Expect(err).To(BeNil()) + + configs, err := agentBaker.GetDistroSigImageConfig(config.SIGConfig, &datamodel.EnvironmentConfig{ + SubscriptionID: config.SubscriptionID, + TenantID: config.TenantID, + Region: cs.Location, + }) + Expect(err).To(BeNil()) + + for _, distro := range allLinuxDistros { + Expect(configs).To(HaveKey(distro)) + config := configs[distro] + Expect(config.ResourceGroup).To(Equal("resourcegroup")) + Expect(config.SubscriptionID).To(Equal("somesubid")) + Expect(config.Version).To(Equal(datamodel.LinuxSIGImageVersion)) + Expect(config.Definition).ToNot(BeEmpty()) + } + + for _, distro := range ubuntuDistros { + config := configs[distro] + Expect(config.Gallery).To(Equal("aksubuntu")) + } + + for _, distro := range marinerDistros { + config := configs[distro] + Expect(config.Gallery).To(Equal("akscblmariner")) + } + + for _, distro := range azureLinuxDistros { + config := configs[distro] + Expect(config.Gallery).To(Equal("aksazurelinux")) + } + }) + + It("should return correct value for all existing distros with linux node image version override", func() { + var ( + ubuntuOverrideVersion = "202402.25.0" + marinerOverrideVersion = "202402.25.1" + azureLinuxOverrideVersion = "202402.25.2" + ) + imageVersionOverrides := map[string]string{} + for _, distro := range ubuntuDistros { + imageVersionOverrides[string(distro)] = ubuntuOverrideVersion + } + for _, distro := range marinerDistros { + imageVersionOverrides[string(distro)] = marinerOverrideVersion + } + for _, distro := range azureLinuxDistros { + imageVersionOverrides[string(distro)] = azureLinuxOverrideVersion + } + toggles.MapToggles = map[string]agenttoggles.MapToggle{ + "linux-node-image-version": func(entity *agenttoggles.Entity) map[string]string { + return imageVersionOverrides + }, + } + + agentBaker, err := NewAgentBaker(toggles) + Expect(err).To(BeNil()) + + configs, err := agentBaker.GetDistroSigImageConfig(config.SIGConfig, &datamodel.EnvironmentConfig{ + SubscriptionID: config.SubscriptionID, + TenantID: config.TenantID, + Region: cs.Location, + }) + Expect(err).To(BeNil()) + + for _, distro := range allLinuxDistros { + Expect(configs).To(HaveKey(distro)) + config := configs[distro] + Expect(config.ResourceGroup).To(Equal("resourcegroup")) + Expect(config.SubscriptionID).To(Equal("somesubid")) + Expect(config.Definition).ToNot(BeEmpty()) + } + + for _, distro := range ubuntuDistros { + config := configs[distro] + Expect(config.Gallery).To(Equal("aksubuntu")) + Expect(config.Version).To(Equal(ubuntuOverrideVersion)) + } + + for _, distro := range marinerDistros { + config := configs[distro] + Expect(config.Gallery).To(Equal("akscblmariner")) + Expect(config.Version).To(Equal(marinerOverrideVersion)) + } + + for _, distro := range azureLinuxDistros { + config := configs[distro] + Expect(config.Gallery).To(Equal("aksazurelinux")) + Expect(config.Version).To(Equal(azureLinuxOverrideVersion)) + } + }) + }) }) diff --git a/pkg/agent/datamodel/sig_config.go b/pkg/agent/datamodel/sig_config.go index 01e98cb2be6..e7f53248d68 100644 --- a/pkg/agent/datamodel/sig_config.go +++ b/pkg/agent/datamodel/sig_config.go @@ -27,6 +27,17 @@ type SIGAzureEnvironmentSpecConfig struct { // TODO(adadilli) add PIR constants as well } +// EnvironmentConfig represents the set of fields required to determine information +// about the customer's environment used to get image config from agentbakersvc APIs. +type EnvironmentConfig struct { + // SubscriptionID is the customer's subscription ID. + SubscriptionID string + // TenantID is the customer's tenant ID. + TenantID string + // Region is the customer's region (e.g. eastus). + Region string +} + // SIGConfig is used to hold configuration parameters to access AKS VHDs stored in a SIG. type SIGConfig struct { TenantID string `json:"tenantID"` @@ -256,6 +267,10 @@ func (d Distro) IsWindowsPIRDistro() bool { return false } +func (d Distro) IsWindowsDistro() bool { + return d.IsWindowsSIGDistro() || d.IsWindowsPIRDistro() +} + // SigImageConfigTemplate represents the SIG image configuration template. // //nolint:musttag // tags can be added if deemed necessary diff --git a/pkg/agent/datamodel/types.go b/pkg/agent/datamodel/types.go index ce403494514..a70e7d52b72 100644 --- a/pkg/agent/datamodel/types.go +++ b/pkg/agent/datamodel/types.go @@ -1623,9 +1623,11 @@ type K8sComponents struct { // //nolint:musttag // tags can be added if deemed necessary type GetLatestSigImageConfigRequest struct { - SIGConfig SIGConfig - Region string - Distro Distro + SIGConfig SIGConfig + SubscriptionID string + TenantID string + Region string + Distro Distro } // NodeBootstrappingConfiguration represents configurations for node bootstrapping. diff --git a/pkg/agent/toggles/fieldnames/fieldnames.go b/pkg/agent/toggles/fieldnames/fieldnames.go new file mode 100644 index 00000000000..e525c998c72 --- /dev/null +++ b/pkg/agent/toggles/fieldnames/fieldnames.go @@ -0,0 +1,7 @@ +package fieldnames + +const ( + SubscriptionID = "subscriptionID" + TenantID = "tenantID" + Region = "region" +) diff --git a/pkg/agent/toggles/toggles.go b/pkg/agent/toggles/toggles.go new file mode 100644 index 00000000000..08629113811 --- /dev/null +++ b/pkg/agent/toggles/toggles.go @@ -0,0 +1,6 @@ +package toggles + +// GetLinuxNodeImageVersion gets the value of the 'linux-node-image-version' toggle as a map. +func (t *Toggles) GetLinuxNodeImageVersion(entity *Entity) map[string]string { + return t.getMap("linux-node-image-version", entity) +} diff --git a/pkg/agent/toggles/toggles_suite_test.go b/pkg/agent/toggles/toggles_suite_test.go new file mode 100644 index 00000000000..35a75c5c94d --- /dev/null +++ b/pkg/agent/toggles/toggles_suite_test.go @@ -0,0 +1,13 @@ +package toggles + +import ( + "testing" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +func TestOverrides(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "toggles suite") +} diff --git a/pkg/agent/toggles/toggles_test.go b/pkg/agent/toggles/toggles_test.go new file mode 100644 index 00000000000..3d8f76f19fe --- /dev/null +++ b/pkg/agent/toggles/toggles_test.go @@ -0,0 +1,82 @@ +package toggles + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +var _ = Describe("toggles tests", func() { + var ( + toggles *Toggles + e = &Entity{Fields: map[string]string{ + "subscriptionId": "sid", + "tenantId": "tid", + }} + ) + + BeforeEach(func() { + toggles = &Toggles{ + MapToggles: map[string]MapToggle{ + "mt1": func(entity *Entity) map[string]string { + return map[string]string{"key": "value"} + }, + "mt2": func(entity *Entity) map[string]string { + return map[string]string{ + "otherKey": "otherValue", + "someOtherKey": "someOtherValue", + } + }, + }, + StringToggles: map[string]StringToggle{ + "st1": func(entity *Entity) string { + return "value" + }, + "st2": func(entity *Entity) string { + return "otherValue" + }, + }, + } + }) + + Context("getMap tests", func() { + When("toggle does not exist", func() { + It("should return the correct default value", func() { + m := toggles.getMap("mt", e) + Expect(m).ToNot(BeNil()) + Expect(m).To(BeEmpty()) + }) + }) + + When("toggle exists", func() { + It("should return the correct value", func() { + m := toggles.getMap("mt1", e) + Expect(len(m)).To(Equal(1)) + Expect(m).To(HaveKeyWithValue("key", "value")) + + m = toggles.getMap("mt2", e) + Expect(len(m)).To(Equal(2)) + Expect(m).To(HaveKeyWithValue("otherKey", "otherValue")) + Expect(m).To(HaveKeyWithValue("someOtherKey", "someOtherValue")) + }) + }) + }) + + Context("getString tests", func() { + When("toggle does not exist", func() { + It("should return the correct default value", func() { + s := toggles.getString("st", e) + Expect(s).To(BeEmpty()) + }) + }) + + When("toggle exists", func() { + It("should return the correct value", func() { + s := toggles.getString("st1", e) + Expect(s).To(Equal("value")) + + s = toggles.getString("st2", e) + Expect(s).To(Equal("otherValue")) + }) + }) + }) +}) diff --git a/pkg/agent/toggles/types.go b/pkg/agent/toggles/types.go new file mode 100644 index 00000000000..7a18c6533b5 --- /dev/null +++ b/pkg/agent/toggles/types.go @@ -0,0 +1,74 @@ +package toggles + +import ( + "github.com/Azure/agentbaker/pkg/agent/datamodel" + "github.com/Azure/agentbaker/pkg/agent/toggles/fieldnames" +) + +// Entity is what we resolve overrides against. It contains any and all fields currently +// used to resolve the set of overrides applied to the agentbakersvc instance. +type Entity struct { + Fields map[string]string +} + +// NewEntity constructs a new Entity from the specified fields. +func NewEntity(fields map[string]string) *Entity { + return &Entity{ + Fields: fields, + } +} + +func NewEntityFromEnvironmentConfig(ctx *datamodel.EnvironmentConfig) *Entity { + return &Entity{ + Fields: map[string]string{ + fieldnames.SubscriptionID: ctx.SubscriptionID, + fieldnames.TenantID: ctx.TenantID, + fieldnames.Region: ctx.Region, + }, + } +} + +func NewEntityFromNodeBootstrappingConfiguration(nbc *datamodel.NodeBootstrappingConfiguration) *Entity { + return &Entity{ + Fields: map[string]string{ + fieldnames.SubscriptionID: nbc.SubscriptionID, + fieldnames.TenantID: nbc.TenantID, + fieldnames.Region: nbc.ContainerService.Location, + }, + } +} + +// MapToggle represents a toggle which resolves a map against a specified Entity. +type MapToggle func(entity *Entity) map[string]string + +// StringToggle represents a toggle which resolves a string against a specified Entity. +type StringToggle func(entity *Entity) string + +// Toggles represents a set of toggles to use within a service context. +type Toggles struct { + // MapToggles is the set of toggles which return map values. + MapToggles map[string]MapToggle + // StringToggles is the set of toggles which return string values + StringToggles map[string]StringToggle +} + +func NewToggles() *Toggles { + return &Toggles{ + MapToggles: make(map[string]MapToggle), + StringToggles: make(map[string]StringToggle), + } +} + +func (t *Toggles) getMap(toggleName string, entity *Entity) map[string]string { + if toggle, ok := t.MapToggles[toggleName]; ok { + return toggle(entity) + } + return map[string]string{} +} + +func (t *Toggles) getString(toggleName string, entity *Entity) string { + if toggle, ok := t.StringToggles[toggleName]; ok { + return toggle(entity) + } + return "" +} From 7e1b5e145aa760124da1cab3e5e28f67b85cc772 Mon Sep 17 00:00:00 2001 From: Cameron Meissner Date: Tue, 26 Mar 2024 09:03:45 -0700 Subject: [PATCH 2/7] chore: logging for testing --- cmd/starter/start.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/cmd/starter/start.go b/cmd/starter/start.go index 2761d9380b1..ffb2ebd7724 100644 --- a/cmd/starter/start.go +++ b/cmd/starter/start.go @@ -18,9 +18,18 @@ func Execute(toggles *agenttoggles.Toggles) { // set toggles options.Toggles = agenttoggles.NewToggles() if toggles != nil { + log.Println("using supplied toggles...") options.Toggles = toggles } + for name := range toggles.MapToggles { + log.Printf("have map toggle %q", name) + } + + for name := range toggles.StringToggles { + log.Printf("have string toggle %q", name) + } + rootCmd.AddCommand(startCmd) startCmd.Flags().StringVar(&options.Addr, "addr", ":8080", "the addr to serve the api on") From 7c7040f45e927660ef522d659e0abce22677999c Mon Sep 17 00:00:00 2001 From: Cameron Meissner Date: Mon, 1 Apr 2024 14:47:23 -0700 Subject: [PATCH 3/7] refactor: opts --- apiserver/apiserver.go | 3 ++ apiserver/getdistrosigimageconfig.go | 6 +++- apiserver/getlatestsigimageconfig.go | 6 +++- apiserver/getnodebootstrapdata.go | 8 +++++- cmd/starter/start.go | 22 ++++----------- fuzz/api/main.go | 2 +- pkg/agent/baker_test.go | 9 +++--- pkg/agent/bakerapi.go | 9 ++++-- pkg/agent/bakerapi_test.go | 42 ++++++++++++++++++---------- pkg/agent/toggles/types.go | 12 +++++--- 10 files changed, 73 insertions(+), 46 deletions(-) diff --git a/apiserver/apiserver.go b/apiserver/apiserver.go index 9518c186abc..0a09c30a881 100644 --- a/apiserver/apiserver.go +++ b/apiserver/apiserver.go @@ -14,6 +14,9 @@ const ( readHeaderTimeoutSeconds = 5 ) +// OptionConfigurator is a function which can configure an Options object. +type OptionConfigurator func(opts *Options) + // Options holds the options for the api server. type Options struct { Addr string diff --git a/apiserver/getdistrosigimageconfig.go b/apiserver/getdistrosigimageconfig.go index f3d65e64ea8..5e9b7dbf567 100644 --- a/apiserver/getdistrosigimageconfig.go +++ b/apiserver/getdistrosigimageconfig.go @@ -26,13 +26,17 @@ func (api *APIServer) GetDistroSigImageConfig(w http.ResponseWriter, r *http.Req return } - agentBaker, err := agent.NewAgentBaker(api.Options.Toggles) + agentBaker, err := agent.NewAgentBaker() if err != nil { log.Println(err.Error()) http.Error(w, err.Error(), http.StatusBadRequest) return } + if api.Options != nil && api.Options.Toggles != nil { + agentBaker = agentBaker.WithToggles(api.Options.Toggles) + } + allDistros, err := agentBaker.GetDistroSigImageConfig(config.SIGConfig, &datamodel.EnvironmentInfo{ SubscriptionID: config.SubscriptionID, TenantID: config.TenantID, diff --git a/apiserver/getlatestsigimageconfig.go b/apiserver/getlatestsigimageconfig.go index 05a6ec805dd..14c149999fa 100644 --- a/apiserver/getlatestsigimageconfig.go +++ b/apiserver/getlatestsigimageconfig.go @@ -26,13 +26,17 @@ func (api *APIServer) GetLatestSigImageConfig(w http.ResponseWriter, r *http.Req return } - agentBaker, err := agent.NewAgentBaker(api.Options.Toggles) + agentBaker, err := agent.NewAgentBaker() if err != nil { log.Println(err.Error()) http.Error(w, err.Error(), http.StatusBadRequest) return } + if api.Options != nil && api.Options.Toggles != nil { + agentBaker = agentBaker.WithToggles(api.Options.Toggles) + } + latestSigConfig, err := agentBaker.GetLatestSigImageConfig(config.SIGConfig, config.Distro, &datamodel.EnvironmentInfo{ SubscriptionID: config.SubscriptionID, TenantID: config.TenantID, diff --git a/apiserver/getnodebootstrapdata.go b/apiserver/getnodebootstrapdata.go index 421a7ad2e1f..60b7586dcc4 100644 --- a/apiserver/getnodebootstrapdata.go +++ b/apiserver/getnodebootstrapdata.go @@ -33,18 +33,24 @@ func (api *APIServer) GetNodeBootstrapData(w http.ResponseWriter, r *http.Reques return } - agentBaker, err := agent.NewAgentBaker(api.Options.Toggles) + agentBaker, err := agent.NewAgentBaker() if err != nil { log.Println(err.Error()) http.Error(w, err.Error(), http.StatusBadRequest) return } + + if api.Options != nil && api.Options.Toggles != nil { + agentBaker = agentBaker.WithToggles(api.Options.Toggles) + } + nodeBootStrapping, err := agentBaker.GetNodeBootstrapping(ctx, &config) if err != nil { log.Println(err.Error()) http.Error(w, err.Error(), http.StatusBadRequest) return } + result, err := json.Marshal(nodeBootStrapping) if err != nil { http.Error(w, err.Error(), http.StatusBadRequest) diff --git a/cmd/starter/start.go b/cmd/starter/start.go index 4c73d97ae4a..37a5eb5a64d 100644 --- a/cmd/starter/start.go +++ b/cmd/starter/start.go @@ -9,30 +9,18 @@ import ( "syscall" "github.com/Azure/agentbaker/apiserver" - agenttoggles "github.com/Azure/agentbaker/pkg/agent/toggles" "github.com/spf13/cobra" ) // Execute adds all child commands to the root command and sets flags appropriately. -func Execute(toggles *agenttoggles.Toggles) { - // set toggles - options.Toggles = agenttoggles.New() - if toggles != nil { - log.Println("using supplied toggles...") - options.Toggles = toggles - } - - for name := range toggles.Maps { - log.Printf("have map toggle %q", name) - } - - for name := range toggles.Maps { - log.Printf("have string toggle %q", name) - } - +func Execute(configurators ...apiserver.OptionConfigurator) { rootCmd.AddCommand(startCmd) startCmd.Flags().StringVar(&options.Addr, "addr", ":8080", "the addr to serve the api on") + for _, configurator := range configurators { + configurator(options) + } + if err := rootCmd.Execute(); err != nil { log.Println(err) os.Exit(1) diff --git a/fuzz/api/main.go b/fuzz/api/main.go index b1ac1b5b8c9..326ad905715 100644 --- a/fuzz/api/main.go +++ b/fuzz/api/main.go @@ -15,7 +15,7 @@ func Fuzz(data []byte) int { return -1 } - baker, err := agent.NewAgentBaker(nil) + baker, err := agent.NewAgentBaker() if err != nil { return -1 } diff --git a/pkg/agent/baker_test.go b/pkg/agent/baker_test.go index a8fc65483f4..5734109fe9f 100644 --- a/pkg/agent/baker_test.go +++ b/pkg/agent/baker_test.go @@ -17,7 +17,6 @@ import ( "strings" "github.com/Azure/agentbaker/pkg/agent/datamodel" - "github.com/Azure/agentbaker/pkg/agent/toggles" "github.com/Azure/go-autorest/autorest/to" "github.com/barkimedes/go-deepcopy" . "github.com/onsi/ginkgo" @@ -221,7 +220,7 @@ var _ = Describe("Assert generated customData and cseCmd", func() { Expect(err).To(BeNil()) // customData - ab, err := NewAgentBaker(toggles.New()) + ab, err := NewAgentBaker() Expect(err).To(BeNil()) nodeBootstrapping, err := ab.GetNodeBootstrapping( context.Background(), @@ -241,7 +240,7 @@ var _ = Describe("Assert generated customData and cseCmd", func() { Expect(customData).To(Equal(string(expectedCustomData))) // CSE - ab, err = NewAgentBaker(toggles.New()) + ab, err = NewAgentBaker() Expect(err).To(BeNil()) nodeBootstrapping, err = ab.GetNodeBootstrapping( context.Background(), @@ -1455,7 +1454,7 @@ var _ = Describe("Assert generated customData and cseCmd for Windows", func() { } // customData - ab, err := NewAgentBaker(toggles.New()) + ab, err := NewAgentBaker() Expect(err).To(BeNil()) nodeBootstrapping, err := ab.GetNodeBootstrapping(context.Background(), config) Expect(err).To(BeNil()) @@ -1475,7 +1474,7 @@ var _ = Describe("Assert generated customData and cseCmd for Windows", func() { Expect(customData).To(Equal(string(expectedCustomData))) // CSE - ab, err = NewAgentBaker(toggles.New()) + ab, err = NewAgentBaker() Expect(err).To(BeNil()) nodeBootstrapping, err = ab.GetNodeBootstrapping(context.Background(), config) Expect(err).To(BeNil()) diff --git a/pkg/agent/bakerapi.go b/pkg/agent/bakerapi.go index 616ba59bf58..32934fe90da 100644 --- a/pkg/agent/bakerapi.go +++ b/pkg/agent/bakerapi.go @@ -18,9 +18,9 @@ type AgentBaker interface { GetDistroSigImageConfig(sigConfig datamodel.SIGConfig, envInfo *datamodel.EnvironmentInfo) (map[datamodel.Distro]datamodel.SigImageConfig, error) } -func NewAgentBaker(toggles *toggles.Toggles) (AgentBaker, error) { +func NewAgentBaker() (*agentBakerImpl, error) { return &agentBakerImpl{ - toggles: toggles, + toggles: toggles.New(), }, nil } @@ -28,6 +28,11 @@ type agentBakerImpl struct { toggles *toggles.Toggles } +func (agentBaker *agentBakerImpl) WithToggles(toggles *toggles.Toggles) *agentBakerImpl { + agentBaker.toggles = toggles + return agentBaker +} + //nolint:revive, nolintlint // ctx is not used, but may be in the future func (agentBaker *agentBakerImpl) GetNodeBootstrapping(ctx context.Context, config *datamodel.NodeBootstrappingConfiguration) (*datamodel.NodeBootstrapping, error) { // validate and fix input before passing config to the template generator. diff --git a/pkg/agent/bakerapi_test.go b/pkg/agent/bakerapi_test.go index 172d154c614..b89bd7bb907 100644 --- a/pkg/agent/bakerapi_test.go +++ b/pkg/agent/bakerapi_test.go @@ -159,8 +159,9 @@ var _ = Describe("AgentBaker API implementation tests", func() { Context("GetNodeBootstrapping", func() { It("should return correct boot strapping data", func() { - agentBaker, err := NewAgentBaker(toggles) + agentBaker, err := NewAgentBaker() Expect(err).NotTo(HaveOccurred()) + agentBaker = agentBaker.WithToggles(toggles) nodeBootStrapping, err := agentBaker.GetNodeBootstrapping(context.Background(), config) Expect(err).NotTo(HaveOccurred()) @@ -190,8 +191,9 @@ var _ = Describe("AgentBaker API implementation tests", func() { }, } - agentBaker, err := NewAgentBaker(toggles) + agentBaker, err := NewAgentBaker() Expect(err).NotTo(HaveOccurred()) + agentBaker = agentBaker.WithToggles(toggles) nodeBootStrapping, err := agentBaker.GetNodeBootstrapping(context.Background(), config) Expect(err).NotTo(HaveOccurred()) @@ -215,8 +217,9 @@ var _ = Describe("AgentBaker API implementation tests", func() { } }, } - agentBaker, err := NewAgentBaker(toggles) + agentBaker, err := NewAgentBaker() Expect(err).NotTo(HaveOccurred()) + agentBaker = agentBaker.WithToggles(toggles) nodeBootStrapping, err := agentBaker.GetNodeBootstrapping(context.Background(), config) Expect(err).NotTo(HaveOccurred()) @@ -242,16 +245,18 @@ var _ = Describe("AgentBaker API implementation tests", func() { config.CloudSpecConfig = cloudSpecConfig config.CloudSpecConfig.CloudName = "UnknownCloud" - agentBaker, err := NewAgentBaker(toggles) + agentBaker, err := NewAgentBaker() Expect(err).NotTo(HaveOccurred()) + agentBaker = agentBaker.WithToggles(toggles) _, err = agentBaker.GetNodeBootstrapping(context.Background(), config) Expect(err).To(HaveOccurred()) }) It("should return an error if distro is neither found in PIR nor found in SIG", func() { config.AgentPoolProfile.Distro = "unknown" - agentBaker, err := NewAgentBaker(toggles) + agentBaker, err := NewAgentBaker() Expect(err).NotTo(HaveOccurred()) + agentBaker = agentBaker.WithToggles(toggles) _, err = agentBaker.GetNodeBootstrapping(context.Background(), config) Expect(err).To(HaveOccurred()) @@ -259,8 +264,9 @@ var _ = Describe("AgentBaker API implementation tests", func() { It("should not return an error for customized image", func() { config.AgentPoolProfile.Distro = datamodel.CustomizedImage - agentBaker, err := NewAgentBaker(toggles) + agentBaker, err := NewAgentBaker() Expect(err).NotTo(HaveOccurred()) + agentBaker = agentBaker.WithToggles(toggles) _, err = agentBaker.GetNodeBootstrapping(context.Background(), config) Expect(err).NotTo(HaveOccurred()) @@ -268,8 +274,9 @@ var _ = Describe("AgentBaker API implementation tests", func() { It("should not return an error for customized kata image", func() { config.AgentPoolProfile.Distro = datamodel.CustomizedImageKata - agentBaker, err := NewAgentBaker(toggles) + agentBaker, err := NewAgentBaker() Expect(err).NotTo(HaveOccurred()) + agentBaker = agentBaker.WithToggles(toggles) _, err = agentBaker.GetNodeBootstrapping(context.Background(), config) Expect(err).NotTo(HaveOccurred()) @@ -277,8 +284,9 @@ var _ = Describe("AgentBaker API implementation tests", func() { It("should not return an error for customized windows image", func() { config.AgentPoolProfile.Distro = datamodel.CustomizedWindowsOSImage - agentBaker, err := NewAgentBaker(toggles) + agentBaker, err := NewAgentBaker() Expect(err).NotTo(HaveOccurred()) + agentBaker = agentBaker.WithToggles(toggles) _, err = agentBaker.GetNodeBootstrapping(context.Background(), config) Expect(err).NotTo(HaveOccurred()) @@ -287,8 +295,9 @@ var _ = Describe("AgentBaker API implementation tests", func() { Context("GetLatestSigImageConfig", func() { It("should return correct value for existing distro", func() { - agentBaker, err := NewAgentBaker(toggles) + agentBaker, err := NewAgentBaker() Expect(err).NotTo(HaveOccurred()) + agentBaker = agentBaker.WithToggles(toggles) sigImageConfig, err := agentBaker.GetLatestSigImageConfig(config.SIGConfig, datamodel.AKSUbuntu1604, &datamodel.EnvironmentInfo{ SubscriptionID: config.SubscriptionID, @@ -311,8 +320,9 @@ var _ = Describe("AgentBaker API implementation tests", func() { } }, } - agentBaker, err := NewAgentBaker(toggles) + agentBaker, err := NewAgentBaker() Expect(err).NotTo(HaveOccurred()) + agentBaker = agentBaker.WithToggles(toggles) sigImageConfig, err := agentBaker.GetLatestSigImageConfig(config.SIGConfig, datamodel.AKSUbuntu1604, &datamodel.EnvironmentInfo{ SubscriptionID: config.SubscriptionID, @@ -335,8 +345,9 @@ var _ = Describe("AgentBaker API implementation tests", func() { } }, } - agentBaker, err := NewAgentBaker(toggles) + agentBaker, err := NewAgentBaker() Expect(err).NotTo(HaveOccurred()) + agentBaker = agentBaker.WithToggles(toggles) sigImageConfig, err := agentBaker.GetLatestSigImageConfig(config.SIGConfig, datamodel.AKSUbuntu1604, &datamodel.EnvironmentInfo{ SubscriptionID: config.SubscriptionID, @@ -352,8 +363,9 @@ var _ = Describe("AgentBaker API implementation tests", func() { }) It("should return error if image config not found for distro", func() { - agentBaker, err := NewAgentBaker(toggles) + agentBaker, err := NewAgentBaker() Expect(err).NotTo(HaveOccurred()) + agentBaker = agentBaker.WithToggles(toggles) _, err = agentBaker.GetLatestSigImageConfig(config.SIGConfig, "unknown", &datamodel.EnvironmentInfo{ SubscriptionID: config.SubscriptionID, @@ -415,8 +427,9 @@ var _ = Describe("AgentBaker API implementation tests", func() { }) It("should return correct value for all existing distros", func() { - agentBaker, err := NewAgentBaker(toggles) + agentBaker, err := NewAgentBaker() Expect(err).To(BeNil()) + agentBaker = agentBaker.WithToggles(toggles) configs, err := agentBaker.GetDistroSigImageConfig(config.SIGConfig, &datamodel.EnvironmentInfo{ SubscriptionID: config.SubscriptionID, @@ -472,8 +485,9 @@ var _ = Describe("AgentBaker API implementation tests", func() { }, } - agentBaker, err := NewAgentBaker(toggles) + agentBaker, err := NewAgentBaker() Expect(err).To(BeNil()) + agentBaker = agentBaker.WithToggles(toggles) configs, err := agentBaker.GetDistroSigImageConfig(config.SIGConfig, &datamodel.EnvironmentInfo{ SubscriptionID: config.SubscriptionID, diff --git a/pkg/agent/toggles/types.go b/pkg/agent/toggles/types.go index c6213645b17..839cb7a002a 100644 --- a/pkg/agent/toggles/types.go +++ b/pkg/agent/toggles/types.go @@ -66,16 +66,20 @@ func New() *Toggles { // getMap attempts to resolve the named map toggle against the specified Entity. func (t *Toggles) getMap(name string, entity *Entity) map[string]string { - if toggle, ok := t.Maps[name]; ok { - return toggle(entity) + if t != nil && t.Maps != nil { + if toggle, ok := t.Maps[name]; ok { + return toggle(entity) + } } return map[string]string{} } // getString attempts to resolve the named string toggle against the specified Entity. func (t *Toggles) getString(name string, entity *Entity) string { - if toggle, ok := t.Strings[name]; ok { - return toggle(entity) + if t != nil && t.Strings != nil { + if toggle, ok := t.Strings[name]; ok { + return toggle(entity) + } } return "" } From d3988bec26e93735797315f8b8dacfaf6fd2b522 Mon Sep 17 00:00:00 2001 From: Cameron Meissner Date: Mon, 1 Apr 2024 14:48:49 -0700 Subject: [PATCH 4/7] chore: cleaning --- cmd/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/main.go b/cmd/main.go index 713be7ad9a2..64df3e1cb6b 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -8,5 +8,5 @@ import ( // NOTE: please make the main.go only contains the starter.Execute(). // that's because other components is reference the same function "starter.Execute()" to start the process. func main() { - starter.Execute(nil) + starter.Execute() } From 13bbd416f90fe0f03e017c37662bf4430baafc3e Mon Sep 17 00:00:00 2001 From: Cameron Meissner Date: Mon, 1 Apr 2024 14:51:38 -0700 Subject: [PATCH 5/7] chore: typecheck --- pkg/agent/bakerapi.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/pkg/agent/bakerapi.go b/pkg/agent/bakerapi.go index 32934fe90da..4a27525827f 100644 --- a/pkg/agent/bakerapi.go +++ b/pkg/agent/bakerapi.go @@ -18,16 +18,18 @@ type AgentBaker interface { GetDistroSigImageConfig(sigConfig datamodel.SIGConfig, envInfo *datamodel.EnvironmentInfo) (map[datamodel.Distro]datamodel.SigImageConfig, error) } +type agentBakerImpl struct { + toggles *toggles.Toggles +} + +var _ AgentBaker = (*agentBakerImpl)(nil) + func NewAgentBaker() (*agentBakerImpl, error) { return &agentBakerImpl{ toggles: toggles.New(), }, nil } -type agentBakerImpl struct { - toggles *toggles.Toggles -} - func (agentBaker *agentBakerImpl) WithToggles(toggles *toggles.Toggles) *agentBakerImpl { agentBaker.toggles = toggles return agentBaker From 4bb426db0ec096d9d21b39d75995288b289d52c7 Mon Sep 17 00:00:00 2001 From: Cameron Meissner Date: Mon, 1 Apr 2024 14:59:37 -0700 Subject: [PATCH 6/7] chore: fix lint --- pkg/agent/bakerapi.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/agent/bakerapi.go b/pkg/agent/bakerapi.go index 4a27525827f..1d540b4af7b 100644 --- a/pkg/agent/bakerapi.go +++ b/pkg/agent/bakerapi.go @@ -24,6 +24,7 @@ type agentBakerImpl struct { var _ AgentBaker = (*agentBakerImpl)(nil) +//nolint:revive // fine to return unexported type due to interface usage func NewAgentBaker() (*agentBakerImpl, error) { return &agentBakerImpl{ toggles: toggles.New(), From 9faaca8ee878a34ebb07cfc80f64bbe4b6bb4da4 Mon Sep 17 00:00:00 2001 From: Cameron Meissner Date: Fri, 5 Apr 2024 12:43:25 -0700 Subject: [PATCH 7/7] chore: more tests and logging --- pkg/agent/toggles/toggles_test.go | 52 +++++++++++++++++++++++++------ pkg/agent/toggles/types.go | 21 ++++++++----- 2 files changed, 55 insertions(+), 18 deletions(-) diff --git a/pkg/agent/toggles/toggles_test.go b/pkg/agent/toggles/toggles_test.go index dd2030a4c17..7df77c15324 100644 --- a/pkg/agent/toggles/toggles_test.go +++ b/pkg/agent/toggles/toggles_test.go @@ -5,17 +5,17 @@ import ( . "github.com/onsi/gomega" ) -var _ = Describe("toggles tests", func() { +var _ = Describe("tgls tests", func() { var ( - toggles *Toggles - e = &Entity{Fields: map[string]string{ + tgls *Toggles + e = &Entity{Fields: map[string]string{ "subscriptionId": "sid", "tenantId": "tid", }} ) BeforeEach(func() { - toggles = &Toggles{ + tgls = &Toggles{ Maps: map[string]MapToggle{ "mt1": func(entity *Entity) map[string]string { return map[string]string{"key": "value"} @@ -39,9 +39,27 @@ var _ = Describe("toggles tests", func() { }) Context("getMap tests", func() { + When("toggles are nil", func() { + It("should return the empty default value", func() { + tgls = nil + m := tgls.getMap("mt", e) + Expect(m).ToNot(BeNil()) + Expect(m).To(BeEmpty()) + }) + }) + + When("map toggles are nil", func() { + It("should return the empty default value", func() { + tgls.Maps = nil + m := tgls.getMap("mt", e) + Expect(m).ToNot(BeNil()) + Expect(m).To(BeEmpty()) + }) + }) + When("toggle does not exist", func() { It("should return the correct default value", func() { - m := toggles.getMap("mt", e) + m := tgls.getMap("mt", e) Expect(m).ToNot(BeNil()) Expect(m).To(BeEmpty()) }) @@ -49,11 +67,11 @@ var _ = Describe("toggles tests", func() { When("toggle exists", func() { It("should return the correct value", func() { - m := toggles.getMap("mt1", e) + m := tgls.getMap("mt1", e) Expect(len(m)).To(Equal(1)) Expect(m).To(HaveKeyWithValue("key", "value")) - m = toggles.getMap("mt2", e) + m = tgls.getMap("mt2", e) Expect(len(m)).To(Equal(2)) Expect(m).To(HaveKeyWithValue("otherKey", "otherValue")) Expect(m).To(HaveKeyWithValue("someOtherKey", "someOtherValue")) @@ -62,19 +80,33 @@ var _ = Describe("toggles tests", func() { }) Context("getString tests", func() { + When("toggles are nil", func() { + It("should return the empty default value", func() { + s := tgls.getString("st", e) + Expect(s).To(BeEmpty()) + }) + }) + + When("string toggles are nil", func() { + It("should return the empty default value", func() { + s := tgls.getString("st", e) + Expect(s).To(BeEmpty()) + }) + }) + When("toggle does not exist", func() { It("should return the correct default value", func() { - s := toggles.getString("st", e) + s := tgls.getString("st", e) Expect(s).To(BeEmpty()) }) }) When("toggle exists", func() { It("should return the correct value", func() { - s := toggles.getString("st1", e) + s := tgls.getString("st1", e) Expect(s).To(Equal("value")) - s = toggles.getString("st2", e) + s = tgls.getString("st2", e) Expect(s).To(Equal("otherValue")) }) }) diff --git a/pkg/agent/toggles/types.go b/pkg/agent/toggles/types.go index 839cb7a002a..419a0585023 100644 --- a/pkg/agent/toggles/types.go +++ b/pkg/agent/toggles/types.go @@ -1,6 +1,8 @@ package toggles import ( + "log" + "github.com/Azure/agentbaker/pkg/agent/datamodel" "github.com/Azure/agentbaker/pkg/agent/toggles/fieldnames" ) @@ -66,20 +68,23 @@ func New() *Toggles { // getMap attempts to resolve the named map toggle against the specified Entity. func (t *Toggles) getMap(name string, entity *Entity) map[string]string { - if t != nil && t.Maps != nil { - if toggle, ok := t.Maps[name]; ok { - return toggle(entity) - } + if t == nil || t.Maps == nil { + log.Printf("map toggles are nil, resolving to default empty map value for toggle: %q", name) + return map[string]string{} + } + if toggle, ok := t.Maps[name]; ok { + return toggle(entity) } return map[string]string{} } // getString attempts to resolve the named string toggle against the specified Entity. func (t *Toggles) getString(name string, entity *Entity) string { - if t != nil && t.Strings != nil { - if toggle, ok := t.Strings[name]; ok { - return toggle(entity) - } + if t == nil || t.Strings == nil { + log.Printf("string toggles are nil, resolving to default empty string value for toggle: %q", name) + } + if toggle, ok := t.Strings[name]; ok { + return toggle(entity) } return "" }