From ebe66b09cba76d57fb47635203d39af541b29c5b Mon Sep 17 00:00:00 2001 From: Yury Kulazhenkov Date: Wed, 22 Nov 2023 18:59:30 +0200 Subject: [PATCH] Read configuration for the app directly from the k8s API This is required to avoid possible issues caused by ConfigMap cache in kubelet. Signed-off-by: Yury Kulazhenkov --- README.md | 56 ++++++++++--- .../app/app.go | 38 ++++++--- .../app/app_test.go | 82 ++++++++++++------- .../app/options/options.go | 28 +++++-- pkg/config/config.go | 11 +-- pkg/config/config_test.go | 36 +++----- 6 files changed, 155 insertions(+), 96 deletions(-) diff --git a/README.md b/README.md index a19ac7d..a8f9e7d 100644 --- a/README.md +++ b/README.md @@ -1,20 +1,29 @@ # network-operator-init-container Init container for NVIDIA Network Operator -The network-operator-init-container container has two required command line arguments: +## Configuration +The network-operator-init-container container has following required command line arguments: - - `--config` path to the configuration file + - `--configmap-name` name of the configmap with configuration for the app + - `--configmap-namespace` namespace of the configmap with configuration for the app - `--node-name` name of the k8s node on which this app runs -The configuration file should be in JSON format: +The ConfigMap should include configuration in JSON format: ``` -{ - "safeDriverLoad": { - "enable": true, - "annotation": "some-annotation" - } -} +apiVersion: v1 +kind: ConfigMap +metadata: + name: ofed-init-container-config + namespace: default +data: + config.json: |- + { + "safeDriverLoad": { + "enable": true, + "annotation": "some-annotation" + } + } ``` - `safeDriverLoad` - contains settings related to safeDriverLoad feature @@ -28,6 +37,25 @@ The container exits with code 0 when the annotation is removed from the Node obj If `safeDriverLoad` feature is disabled then the container will immediately exit with code 0. +### Required permissions + +``` +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: network-operator-init-container +rules: + - apiGroups: [""] + resources: ["nodes"] + verbs: ["get", "list", "patch", "watch", "update"] + - apiGroups: [""] + resources: ["configmaps"] + verbs: ["get", "list"] + +``` + +## Command line arguments + ``` NVIDIA Network Operator init container @@ -36,8 +64,12 @@ Usage: Config flags: - --config string - path to the configuration file + --configmap-key string + key inside the configmap with configuration for the app (default "config.json") + --configmap-name string + name of the configmap with configuration for the app + --configmap-namespace string + namespace of the configmap with configuration for the app --node-name string name of the k8s node on which this app runs @@ -70,4 +102,4 @@ Kubernetes flags: --kubeconfig string Paths to a kubeconfig. Only required if out-of-cluster. -``` \ No newline at end of file +``` diff --git a/cmd/network-operator-init-container/app/app.go b/cmd/network-operator-init-container/app/app.go index 7ad166c..d2a7189 100644 --- a/cmd/network-operator-init-container/app/app.go +++ b/cmd/network-operator-init-container/app/app.go @@ -97,18 +97,6 @@ func RunNetworkOperatorInitContainer(ctx context.Context, config *rest.Config, o "Options", opts, "Version", version.GetVersionString()) ctrl.SetLogger(logger) - initContCfg, err := configPgk.FromFile(opts.ConfigPath) - if err != nil { - logger.Error(err, "failed to read configuration") - return err - } - logger.Info("network-operator-init-container configuration", "config", initContCfg.String()) - - if !initContCfg.SafeDriverLoad.Enable { - logger.Info("safe driver loading is disabled, exit") - return nil - } - mgr, err := ctrl.NewManager(config, ctrl.Options{ Metrics: metricsserver.Options{BindAddress: "0"}, Cache: cache.Options{ @@ -117,7 +105,7 @@ func RunNetworkOperatorInitContainer(ctx context.Context, config *rest.Config, o fmt.Sprintf("metadata.name=%s", opts.NodeName))}}}, }) if err != nil { - logger.Error(err, "unable to start manager") + logger.Error(err, "unable to create manager") return err } @@ -128,6 +116,30 @@ func RunNetworkOperatorInitContainer(ctx context.Context, config *rest.Config, o return err } + confConfigMap := &corev1.ConfigMap{} + + err = k8sClient.Get(ctx, client.ObjectKey{ + Name: opts.ConfigMapName, + Namespace: opts.ConfigMapNamespace, + }, confConfigMap) + + if err != nil { + logger.Error(err, "failed to read config map with configuration") + return err + } + + initContCfg, err := configPgk.Load(confConfigMap.Data[opts.ConfigMapKey]) + if err != nil { + logger.Error(err, "failed to read configuration") + return err + } + logger.Info("network-operator-init-container configuration", "config", initContCfg.String()) + + if !initContCfg.SafeDriverLoad.Enable { + logger.Info("safe driver loading is disabled, exit") + return nil + } + errCh := make(chan error, 1) if err = (&NodeReconciler{ diff --git a/cmd/network-operator-init-container/app/app_test.go b/cmd/network-operator-init-container/app/app_test.go index 269a5cc..2145180 100644 --- a/cmd/network-operator-init-container/app/app_test.go +++ b/cmd/network-operator-init-container/app/app_test.go @@ -14,13 +14,13 @@ package app_test import ( + "context" "fmt" - "os" - "path/filepath" "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + apiErrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/json" "sigs.k8s.io/controller-runtime/pkg/client" @@ -34,8 +34,11 @@ import ( ) const ( - testNodeName = "node1" - testAnnotation = "foo.bar/spam" + testConfigMapName = "test" + testConfigMapNamespace = "default" + testConfigMapKey = "conf" + testNodeName = "node1" + testAnnotation = "foo.bar/spam" ) func createNode(name string) *corev1.Node { @@ -44,44 +47,67 @@ func createNode(name string) *corev1.Node { return node } -func createConfig(path string, cfg configPgk.Config) { +func createConfig(cfg configPgk.Config) { data, err := json.Marshal(cfg) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - ExpectWithOffset(1, os.WriteFile(path, data, 0x744)) + err = k8sClient.Create(ctx, &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: testConfigMapName, Namespace: testConfigMapNamespace}, + Data: map[string]string{testConfigMapKey: string(data)}, + }) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) +} + +func newOpts() *options.Options { + return &options.Options{ + ConfigMapName: testConfigMapName, + ConfigMapNamespace: testConfigMapNamespace, + ConfigMapKey: testConfigMapKey, + } } var _ = Describe("Init container", func() { var ( - configPath string + testCtx context.Context + testCFunc context.CancelFunc ) + BeforeEach(func() { - configPath = filepath.Join(GinkgoT().TempDir(), "config") + testCtx, testCFunc = context.WithCancel(ctx) + }) + + AfterEach(func() { + err := k8sClient.Delete(ctx, &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: testConfigMapName, Namespace: testConfigMapNamespace}, + }) + if !apiErrors.IsNotFound(err) { + Expect(err).NotTo(HaveOccurred()) + } + testCFunc() }) It("Succeed", func() { testDone := make(chan interface{}) go func() { defer close(testDone) defer GinkgoRecover() - opts := options.New() + opts := newOpts() opts.NodeName = testNodeName - opts.ConfigPath = configPath - createConfig(configPath, configPgk.Config{SafeDriverLoad: configPgk.SafeDriverLoadConfig{ + createConfig(configPgk.Config{SafeDriverLoad: configPgk.SafeDriverLoadConfig{ Enable: true, Annotation: testAnnotation, }}) var err error appExit := make(chan interface{}) go func() { - err = app.RunNetworkOperatorInitContainer(ctx, cfg, opts) + err = app.RunNetworkOperatorInitContainer(testCtx, cfg, opts) close(appExit) }() node := &corev1.Node{} Eventually(func(g Gomega) { - g.Expect(k8sClient.Get(ctx, types.NamespacedName{Name: testNodeName}, node)).NotTo(HaveOccurred()) + g.Expect(k8sClient.Get(testCtx, types.NamespacedName{Name: testNodeName}, node)).NotTo(HaveOccurred()) g.Expect(node.GetAnnotations()[testAnnotation]).NotTo(BeEmpty()) }, 30, 1).Should(Succeed()) // remove annotation - Expect(k8sClient.Patch(ctx, node, client.RawPatch( + Expect(k8sClient.Patch(testCtx, node, client.RawPatch( types.MergePatchType, []byte( fmt.Sprintf(`{"metadata":{"annotations":{%q: null}}}`, testAnnotation))))).NotTo(HaveOccurred()) @@ -95,17 +121,16 @@ var _ = Describe("Init container", func() { go func() { defer close(testDone) defer GinkgoRecover() - opts := options.New() + opts := newOpts() opts.NodeName = "unknown-node" - opts.ConfigPath = configPath - createConfig(configPath, configPgk.Config{SafeDriverLoad: configPgk.SafeDriverLoadConfig{ + createConfig(configPgk.Config{SafeDriverLoad: configPgk.SafeDriverLoadConfig{ Enable: true, Annotation: testAnnotation, }}) var err error appExit := make(chan interface{}) go func() { - err = app.RunNetworkOperatorInitContainer(ctx, cfg, opts) + err = app.RunNetworkOperatorInitContainer(testCtx, cfg, opts) close(appExit) }() Eventually(appExit, 30, 1).Should(BeClosed()) @@ -118,20 +143,19 @@ var _ = Describe("Init container", func() { go func() { defer close(testDone) defer GinkgoRecover() - opts := options.New() + opts := newOpts() opts.NodeName = testNodeName - opts.ConfigPath = configPath - createConfig(configPath, configPgk.Config{SafeDriverLoad: configPgk.SafeDriverLoadConfig{ + createConfig(configPgk.Config{SafeDriverLoad: configPgk.SafeDriverLoadConfig{ Enable: true, Annotation: testAnnotation, }}) var err error appExit := make(chan interface{}) go func() { - err = app.RunNetworkOperatorInitContainer(ctx, cfg, opts) + err = app.RunNetworkOperatorInitContainer(testCtx, cfg, opts) close(appExit) }() - cFunc() + testCFunc() Eventually(appExit, 30, 1).Should(BeClosed()) Expect(err).To(HaveOccurred()) }() @@ -142,13 +166,12 @@ var _ = Describe("Init container", func() { go func() { defer close(testDone) defer GinkgoRecover() - opts := options.New() + opts := newOpts() opts.NodeName = "unknown-node" - opts.ConfigPath = configPath var err error appExit := make(chan interface{}) go func() { - err = app.RunNetworkOperatorInitContainer(ctx, cfg, opts) + err = app.RunNetworkOperatorInitContainer(testCtx, cfg, opts) close(appExit) }() Eventually(appExit, 30, 1).Should(BeClosed()) @@ -161,16 +184,15 @@ var _ = Describe("Init container", func() { go func() { defer close(testDone) defer GinkgoRecover() - opts := options.New() + opts := newOpts() opts.NodeName = testNodeName - opts.ConfigPath = configPath - createConfig(configPath, configPgk.Config{SafeDriverLoad: configPgk.SafeDriverLoadConfig{ + createConfig(configPgk.Config{SafeDriverLoad: configPgk.SafeDriverLoadConfig{ Enable: false, }}) var err error appExit := make(chan interface{}) go func() { - err = app.RunNetworkOperatorInitContainer(ctx, cfg, opts) + err = app.RunNetworkOperatorInitContainer(testCtx, cfg, opts) close(appExit) }() Eventually(appExit, 30, 1).Should(BeClosed()) diff --git a/cmd/network-operator-init-container/app/options/options.go b/cmd/network-operator-init-container/app/options/options.go index a8fe6d1..0bac7e9 100644 --- a/cmd/network-operator-init-container/app/options/options.go +++ b/cmd/network-operator-init-container/app/options/options.go @@ -32,9 +32,11 @@ func New() *Options { // Options contains application options type Options struct { - NodeName string - ConfigPath string - LogConfig *logsapi.LoggingConfiguration + NodeName string + ConfigMapName string + ConfigMapNamespace string + ConfigMapKey string + LogConfig *logsapi.LoggingConfiguration } // AddNamedFlagSets returns FlagSet for Options @@ -42,8 +44,12 @@ func (o *Options) AddNamedFlagSets(sharedFS *cliflag.NamedFlagSets) { configFS := sharedFS.FlagSet("Config") configFS.StringVar(&o.NodeName, "node-name", "", "name of the k8s node on which this app runs") - configFS.StringVar(&o.ConfigPath, "config", "", - "path to the configuration file") + configFS.StringVar(&o.ConfigMapName, "configmap-name", "", + "name of the configmap with configuration for the app") + configFS.StringVar(&o.ConfigMapNamespace, "configmap-namespace", "", + "namespace of the configmap with configuration for the app") + configFS.StringVar(&o.ConfigMapKey, "configmap-key", "config.json", + "key inside the configmap with configuration for the app") logFS := sharedFS.FlagSet("Logging") logsapi.AddFlags(o.LogConfig, logFS) @@ -68,8 +74,16 @@ func (o *Options) Validate() error { return fmt.Errorf("node-name is required parameter") } - if o.ConfigPath == "" { - return fmt.Errorf("config is required parameter") + if o.ConfigMapName == "" { + return fmt.Errorf("configmap-name is required parameter") + } + + if o.ConfigMapNamespace == "" { + return fmt.Errorf("configmap-namespace is required parameter") + } + + if o.ConfigMapKey == "" { + return fmt.Errorf("configmap-key is required parameter") } if err = logsapi.ValidateAndApply(o.LogConfig, nil); err != nil { diff --git a/pkg/config/config.go b/pkg/config/config.go index dcf0d6d..d9ce0e4 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -15,19 +15,14 @@ package config import ( "fmt" - "os" "k8s.io/apimachinery/pkg/util/json" ) -// FromFile reads configuration from the file -func FromFile(path string) (*Config, error) { - data, err := os.ReadFile(path) - if err != nil { - return nil, fmt.Errorf("failed to read data from the provided file: %v", err) - } +// Load parse configuration from the provided string +func Load(config string) (*Config, error) { cfg := &Config{} - if err := json.Unmarshal(data, cfg); err != nil { + if err := json.Unmarshal([]byte(config), cfg); err != nil { return nil, fmt.Errorf("failed to unmarshal configuration: %v", err) } if err := cfg.Validate(); err != nil { diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 8df29ee..90d27f5 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -15,8 +15,6 @@ package config_test import ( "encoding/json" - "os" - "path/filepath" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -24,51 +22,37 @@ import ( configPgk "github.com/Mellanox/network-operator-init-container/pkg/config" ) -func createConfig(path string, cfg configPgk.Config) { +func createConfig(cfg configPgk.Config) string { data, err := json.Marshal(cfg) ExpectWithOffset(1, err).NotTo(HaveOccurred()) - ExpectWithOffset(1, os.WriteFile(path, data, 0x744)) + return string(data) } var _ = Describe("Config test", func() { - var ( - configPath string - ) - BeforeEach(func() { - configPath = filepath.Join(GinkgoT().TempDir(), "config") - }) It("Valid - safeDriverLoad disabled", func() { - createConfig(configPath, configPgk.Config{SafeDriverLoad: configPgk.SafeDriverLoadConfig{ + cfg, err := configPgk.Load(createConfig(configPgk.Config{SafeDriverLoad: configPgk.SafeDriverLoadConfig{ Enable: false, - }}) - cfg, err := configPgk.FromFile(configPath) + }})) Expect(err).NotTo(HaveOccurred()) Expect(cfg.SafeDriverLoad.Enable).To(BeFalse()) }) It("Valid - safeDriverLoad enabled", func() { - createConfig(configPath, configPgk.Config{SafeDriverLoad: configPgk.SafeDriverLoadConfig{ + cfg, err := configPgk.Load(createConfig(configPgk.Config{SafeDriverLoad: configPgk.SafeDriverLoadConfig{ Enable: true, Annotation: "something", - }}) - cfg, err := configPgk.FromFile(configPath) + }})) Expect(err).NotTo(HaveOccurred()) Expect(cfg.SafeDriverLoad.Enable).To(BeTrue()) Expect(cfg.SafeDriverLoad.Annotation).To(Equal("something")) }) - It("Failed to read config", func() { - _, err := configPgk.FromFile(configPath) - Expect(err).To(HaveOccurred()) - }) It("Failed to unmarshal config", func() { - Expect(os.WriteFile(configPath, []byte("invalid\""), 0x744)).NotTo(HaveOccurred()) - _, err := configPgk.FromFile(configPath) + _, err := configPgk.Load("invalid\"") Expect(err).To(HaveOccurred()) }) - It("Logical validation failed", func() { - createConfig(configPath, configPgk.Config{SafeDriverLoad: configPgk.SafeDriverLoadConfig{ + It("Logical validation failed - no annotation", func() { + _, err := configPgk.Load(createConfig(configPgk.Config{SafeDriverLoad: configPgk.SafeDriverLoadConfig{ Enable: true, - }}) - _, err := configPgk.FromFile(configPath) + }})) Expect(err).To(HaveOccurred()) }) })