From 4bc3d6c13e15a2f1e3af800d259730a0e76a5048 Mon Sep 17 00:00:00 2001 From: hxcGit Date: Wed, 1 Mar 2023 00:15:10 +0200 Subject: [PATCH] modify the way to obtain flags Signed-off-by: hxcGit --- .../static-pod-upgrade.go | 57 ++++++++++++------ pkg/static-pod-upgrade/upgrade.go | 59 +++++++------------ pkg/static-pod-upgrade/upgrade_test.go | 28 ++++----- 3 files changed, 73 insertions(+), 71 deletions(-) diff --git a/cmd/yurt-static-pod-upgrade/static-pod-upgrade.go b/cmd/yurt-static-pod-upgrade/static-pod-upgrade.go index 581722a09a3..a32241bf738 100644 --- a/cmd/yurt-static-pod-upgrade/static-pod-upgrade.go +++ b/cmd/yurt-static-pod-upgrade/static-pod-upgrade.go @@ -20,18 +20,27 @@ import ( "fmt" "math/rand" "os" + "reflect" "time" "github.com/spf13/cobra" "github.com/spf13/pflag" - "github.com/spf13/viper" "k8s.io/klog/v2" "github.com/openyurtio/openyurt/pkg/projectinfo" upgrade "github.com/openyurtio/openyurt/pkg/static-pod-upgrade" ) +type Config struct { + Name string + Namespace string + Manifest string + Hash string + Mode string +} + func main() { + cfg := &Config{} rand.Seed(time.Now().UnixNano()) version := fmt.Sprintf("%#v", projectinfo.Get()) cmd := &cobra.Command{ @@ -43,7 +52,7 @@ func main() { klog.Infof("FLAG: --%s=%q", flag.Name, flag.Value) }) - if err := upgrade.Validate(); err != nil { + if err := cfg.validate(); err != nil { klog.Fatalf("Fail to validate yurt static pod upgrade args, %v", err) } @@ -52,12 +61,12 @@ func main() { klog.Fatalf("Fail to get kubernetes client, %v", err) } - ctrl, err := upgrade.New(c) + ctrl, err := upgrade.New(c, cfg.Name, cfg.Namespace, cfg.Manifest, cfg.Hash, cfg.Mode) if err != nil { - klog.Fatal("Fail to create static-pod-upgrade controller, %v", err) + klog.Fatalf("Fail to create static-pod-upgrade controller, %v", err) } - if err := ctrl.Upgrade(); err != nil { + if err = ctrl.Upgrade(); err != nil { klog.Fatalf("Fail to upgrade static pod, %v", err) } klog.Info("Static pod upgrade Success") @@ -65,22 +74,36 @@ func main() { Version: version, } - addFlags(cmd) - - if err := viper.BindPFlags(cmd.Flags()); err != nil { - os.Exit(1) - } + cfg.addFlags(cmd) if err := cmd.Execute(); err != nil { os.Exit(1) } } -func addFlags(cmd *cobra.Command) { - cmd.Flags().String("kubeconfig", "", "The path to the kubeconfig file") - cmd.Flags().String("name", "", "The name of static pod which needs be upgraded") - cmd.Flags().String("namespace", "", "The namespace of static pod which needs be upgraded") - cmd.Flags().String("manifest", "", "The manifest file name of static pod which needs be upgraded") - cmd.Flags().String("hash", "", "The hash value of new static pod specification") - cmd.Flags().String("mode", "", "The upgrade mode which is used") +func (c *Config) addFlags(cmd *cobra.Command) { + cmd.Flags().StringVar(&c.Name, "name", "", "The name of static pod which needs be upgraded") + cmd.Flags().StringVar(&c.Namespace, "namespace", "", "The namespace of static pod which needs be upgraded") + cmd.Flags().StringVar(&c.Manifest, "manifest", "", "The manifest file name of static pod which needs be upgraded") + cmd.Flags().StringVar(&c.Hash, "hash", "", "The hash value of new static pod specification") + cmd.Flags().StringVar(&c.Mode, "mode", "", "The upgrade mode which is used") +} + +// Validate check if all the required arguments are valid +func (c *Config) validate() error { + v := reflect.ValueOf(*c) + + for i := 0; i < v.NumField(); i++ { + field := v.Field(i) + if field.Len() == 0 { + return fmt.Errorf("arg %s is empty", v.Type().Field(i).Name) + } + } + + // TODO: use constant value of static-pod controller + if c.Mode != "auto" && c.Mode != "ota" { + return fmt.Errorf("only support auto or ota upgrade mode") + } + + return nil } diff --git a/pkg/static-pod-upgrade/upgrade.go b/pkg/static-pod-upgrade/upgrade.go index a2d99787c49..246f61e8b7b 100644 --- a/pkg/static-pod-upgrade/upgrade.go +++ b/pkg/static-pod-upgrade/upgrade.go @@ -23,7 +23,6 @@ import ( "path/filepath" "time" - "github.com/spf13/viper" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/validation" "k8s.io/client-go/kubernetes" @@ -35,7 +34,7 @@ const ( DefaultUpgradeDir = "openyurtio-upgrade" defaultStaticPodRunningCheckTimeout = 2 * time.Minute - // TODO: use static-pod-upgrade's constant value + // TODO: use constant value of static-pod controller OTA = "ota" Auto = "auto" ) @@ -43,11 +42,9 @@ const ( var ( DefaultConfigmapPath = "/data" DefaultManifestPath = "/etc/kubernetes/manifests" - - RequiredField = []string{"name", "namespace", "hash", "mode"} ) -type UpgradeController struct { +type Controller struct { client kubernetes.Interface // Name of static pod @@ -71,20 +68,14 @@ type UpgradeController struct { upgradeManifestPath string } -func New(client kubernetes.Interface) (*UpgradeController, error) { - ctrl := &UpgradeController{ - client: client, - } - - ctrl.name = viper.GetString("name") - ctrl.namespace = viper.GetString("namespace") - ctrl.manifest = viper.GetString("manifest") - ctrl.hash = viper.GetString("hash") - ctrl.upgradeMode = viper.GetString("mode") - - // Manifest file name is optional. If not set, default is static pod name - if len(ctrl.manifest) == 0 { - ctrl.manifest = ctrl.name +func New(client kubernetes.Interface, name, namespace, manifest, hash, mode string) (*Controller, error) { + ctrl := &Controller{ + client: client, + name: name, + namespace: namespace, + manifest: manifest, + hash: hash, + upgradeMode: mode, } ctrl.manifestPath = filepath.Join(DefaultManifestPath, WithYamlSuffix(ctrl.manifest)) @@ -95,7 +86,7 @@ func New(client kubernetes.Interface) (*UpgradeController, error) { return ctrl, nil } -func (ctrl *UpgradeController) Upgrade() error { +func (ctrl *Controller) Upgrade() error { // 1. Check the target static pod exist if err := ctrl.checkStaticPodExist(); err != nil { return err @@ -126,7 +117,7 @@ func (ctrl *UpgradeController) Upgrade() error { return nil } -func (ctrl *UpgradeController) AutoUpgrade() error { +func (ctrl *Controller) AutoUpgrade() error { // (1) Back up the old manifest in case of upgrade failure if err := ctrl.backupManifest(); err != nil { return err @@ -153,7 +144,7 @@ func (ctrl *UpgradeController) AutoUpgrade() error { } // In ota mode, just need to set the latest upgrade manifest version -func (ctrl *UpgradeController) OTAUpgrade() error { +func (ctrl *Controller) OTAUpgrade() error { if err := ctrl.setLatestManifestHash(); err != nil { return err } @@ -162,7 +153,7 @@ func (ctrl *UpgradeController) OTAUpgrade() error { } // checkStaticPodExist check if the target static pod exist in cluster -func (ctrl *UpgradeController) checkStaticPodExist() error { +func (ctrl *Controller) checkStaticPodExist() error { if errs := validation.IsDNS1123Subdomain(ctrl.name); len(errs) > 0 { return fmt.Errorf("pod name %s is invalid: %v", ctrl.name, errs) } @@ -174,7 +165,7 @@ func (ctrl *UpgradeController) checkStaticPodExist() error { } // checkManifestFileExist check if the specified files exist -func (ctrl *UpgradeController) checkManifestFileExist() error { +func (ctrl *Controller) checkManifestFileExist() error { check := []string{ctrl.manifestPath, ctrl.configMapDataPath} for _, c := range check { _, err := os.Stat(c) @@ -189,7 +180,7 @@ func (ctrl *UpgradeController) checkManifestFileExist() error { // prepareManifest move the latest manifest to DefaultUpgradeDir and set `.upgrade` suffix // TODO: In kubernetes when mount configmap file to the sub path of hostpath mount, it will not be persistent // TODO: Init configmap(latest manifest) to a default place and move it to `DefaultUpgradeDir` to save it persistent -func (ctrl *UpgradeController) prepareManifest() error { +func (ctrl *Controller) prepareManifest() error { // Make sure upgrade dir exist if _, err := os.Stat(filepath.Join(DefaultManifestPath, DefaultUpgradeDir)); os.IsNotExist(err) { if err = os.Mkdir(filepath.Join(DefaultManifestPath, DefaultUpgradeDir), 0755); err != nil { @@ -201,25 +192,25 @@ func (ctrl *UpgradeController) prepareManifest() error { } // backUpManifest backup the old manifest in order to roll back when errors occur -func (ctrl *UpgradeController) backupManifest() error { +func (ctrl *Controller) backupManifest() error { return CopyFile(ctrl.manifestPath, ctrl.bakManifestPath) } // replaceManifest replace old manifest with the latest one, it achieves static pod upgrade -func (ctrl *UpgradeController) replaceManifest() error { +func (ctrl *Controller) replaceManifest() error { return CopyFile(ctrl.upgradeManifestPath, ctrl.manifestPath) } // verify make sure the latest static pod is running // return false when the latest static pod failed or check status time out -func (ctrl *UpgradeController) verify() (bool, error) { +func (ctrl *Controller) verify() (bool, error) { return WaitForPodRunning(ctrl.client, ctrl.name, ctrl.namespace, ctrl.hash, defaultStaticPodRunningCheckTimeout) } // setLatestManifestHash set the latest manifest hash value to target static pod annotation // TODO: In ota mode, it's hard for controller to check whether the latest manifest file has been issued to nodes // TODO: Use annotation `openyurt.io/ota-manifest-version` to indicate the version of manifest issued to nodes -func (ctrl *UpgradeController) setLatestManifestHash() error { +func (ctrl *Controller) setLatestManifestHash() error { pod, err := ctrl.client.CoreV1().Pods(ctrl.namespace).Get(context.TODO(), ctrl.name, metav1.GetOptions{}) if err != nil { return err @@ -229,16 +220,6 @@ func (ctrl *UpgradeController) setLatestManifestHash() error { return err } -// Validate check if all the required arguments are valid -func Validate() error { - for _, r := range RequiredField { - if v := viper.GetString(r); len(v) == 0 { - return fmt.Errorf("arg %s is empty", r) - } - } - return nil -} - func GetClient() (kubernetes.Interface, error) { config, err := rest.InClusterConfig() if err != nil { diff --git a/pkg/static-pod-upgrade/upgrade_test.go b/pkg/static-pod-upgrade/upgrade_test.go index 7e6204c43ed..deb1567b5a4 100644 --- a/pkg/static-pod-upgrade/upgrade_test.go +++ b/pkg/static-pod-upgrade/upgrade_test.go @@ -22,7 +22,6 @@ import ( "path/filepath" "testing" - "github.com/spf13/viper" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/watch" @@ -32,22 +31,23 @@ import ( "github.com/openyurtio/openyurt/pkg/yurthub/util" ) +const ( + TestPodName = "nginx" + TestHashValue = "789c7f9f47" + TestManifest = "manifest" +) + func Test(t *testing.T) { // Temporarily modify the manifest path in order to test DefaultManifestPath = t.TempDir() DefaultConfigmapPath = t.TempDir() - _, _ = os.Create(filepath.Join(DefaultManifestPath, WithYamlSuffix("nginxManifest"))) - _, _ = os.Create(filepath.Join(DefaultConfigmapPath, "nginxManifest")) - - viper.Set("name", "nginx-node") - viper.Set("namespace", "default") - viper.Set("manifest", "nginxManifest") - viper.Set("hash", "789c7f9f47") + _, _ = os.Create(filepath.Join(DefaultManifestPath, WithYamlSuffix(TestManifest))) + _, _ = os.Create(filepath.Join(DefaultConfigmapPath, TestManifest)) runningStaticPod := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ - Name: "nginx-node", - Namespace: "default", + Name: TestPodName, + Namespace: metav1.NamespaceDefault, }, Status: corev1.PodStatus{ Phase: corev1.PodRunning, @@ -62,7 +62,7 @@ func Test(t *testing.T) { */ if mode == "auto" { runningStaticPod.Annotations = map[string]string{ - StaticPodHashAnnotation: "789c7f9f47", + StaticPodHashAnnotation: TestHashValue, } } c := fake.NewSimpleClientset(runningStaticPod) @@ -73,12 +73,10 @@ func Test(t *testing.T) { watcher.Add(runningStaticPod) }() - viper.Set("mode", mode) - /* 2. Test */ - ctrl, err := New(c) + ctrl, err := New(c, TestPodName, metav1.NamespaceDefault, TestManifest, TestHashValue, mode) if err != nil { t.Errorf("Fail to get upgrade controller, %v", err) } @@ -105,7 +103,7 @@ func Test(t *testing.T) { t.Errorf("Fail to get the running static pod, %v", err) } - if pod.Annotations[OTALatestManifestAnnotation] != "789c7f9f47" { + if pod.Annotations[OTALatestManifestAnnotation] != TestHashValue { t.Errorf("Fail to verify hash annotation for ota upgrade, %v", err) } }