diff --git a/Makefile b/Makefile index fb385f6d4fb03d526cc4cfba9073502a286efc6a..33e82f058cb386e702d396a5be917455c168996b 100644 --- a/Makefile +++ b/Makefile @@ -123,7 +123,7 @@ kustomize: .PHONY: test test: manifests fmt vet envtest ## Run tests. - KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test ./... -coverprofile cover.out + KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test ./... -race -count=1 -timeout=300s -cover -gcflags=all=-l .PHONY: envtest envtest: $(ENVTEST) ## Download envtest-setup locally if necessary. diff --git a/cmd/agent/server/config.go b/cmd/agent/server/config.go index b4385e268c50f771c1c9e49cb5ded6924c78d389..a1662003f9a9e51f495f3dac5c4d832684da4fc6 100644 --- a/cmd/agent/server/config.go +++ b/cmd/agent/server/config.go @@ -89,7 +89,10 @@ func (k KerSysctlPersist) SetConfig(config *agent.SysConfig) error { } // GrubCmdline represents grub.cmdline configuration -type GrubCmdline struct{} +type GrubCmdline struct { + // it represents which partition the user want to configure + isCurPartition bool +} // SetConfig sets grub.cmdline configuration func (g GrubCmdline) SetConfig(config *agent.SysConfig) error { @@ -102,7 +105,12 @@ func (g GrubCmdline) SetConfig(config *agent.SysConfig) error { if !fileExist { return fmt.Errorf("failed to find grub.cfg %s", getGrubCfgPath()) } - lines, err := getAndSetGrubCfg(config.Contents) + configPartition, err := getConfigPartition(g.isCurPartition) + if err != nil { + logrus.Errorf("Failed to get config partition: %v", err) + return err + } + lines, err := getAndSetGrubCfg(config.Contents, configPartition) if err != nil { logrus.Errorf("Failed to set grub configs: %v", err) return err @@ -113,7 +121,25 @@ func (g GrubCmdline) SetConfig(config *agent.SysConfig) error { return nil } -func getAndSetGrubCfg(expectConfigs map[string]*agent.KeyInfo) ([]string, error) { +// getConfigPartition return false if the user want to configure partition A, +// return true if the user want to configure partition B +func getConfigPartition(isCurPartition bool) (bool, error) { + partA, partB, err := getRootfsDisks() + if err != nil { + return false, err + } + _, next, err := getNextPart(partA, partB) + if err != nil { + return false, err + } + var flag bool + if next == "B" { + flag = true + } + return isCurPartition != flag, nil +} + +func getAndSetGrubCfg(expectConfigs map[string]*agent.KeyInfo, configPartition bool) ([]string, error) { file, err := os.OpenFile(getGrubCfgPath(), os.O_RDWR, defaultGrubCfgPermission) if err != nil { return []string{}, err @@ -127,14 +153,18 @@ func getAndSetGrubCfg(expectConfigs map[string]*agent.KeyInfo) ([]string, error) } var lines []string + var matchCount bool configScanner := bufio.NewScanner(file) for configScanner.Scan() { line := configScanner.Text() if r.MatchString(line) { - line, err = modifyLinuxCfg(expectConfigs, line) - if err != nil { - return []string{}, fmt.Errorf("error modify grub.cfg %v", err) + if matchCount == configPartition { + line, err = modifyLinuxCfg(expectConfigs, line) + if err != nil { + return []string{}, fmt.Errorf("error modify grub.cfg %v", err) + } } + matchCount = true } lines = append(lines, line) } @@ -221,8 +251,8 @@ func ConfigFactoryTemplate(configType string, config *agent.SysConfig) error { doConfig.Do(func() { configTemplate[KernelSysctlName.String()] = new(KernelSysctl) configTemplate[KerSysctlPersistName.String()] = new(KerSysctlPersist) - configTemplate[GrubCmdlineName.String()] = new(GrubCmdline) - + configTemplate[GrubCmdlineCurName.String()] = &GrubCmdline{isCurPartition: true} + configTemplate[GrubCmdlineNextName.String()] = &GrubCmdline{isCurPartition: false} }) if _, ok := configTemplate[configType]; ok { return configTemplate[configType].SetConfig(config) diff --git a/cmd/agent/server/config_test.go b/cmd/agent/server/config_test.go index 56b3a47a61b7056134cdc1f958099bbcbd9a43b8..725d1f8f6cd7e3f1981e32d94e13db596bfe4b7a 100644 --- a/cmd/agent/server/config_test.go +++ b/cmd/agent/server/config_test.go @@ -203,8 +203,8 @@ menuentry 'B' --class KubeOS --class gnu-linux --class gnu --class os --unrestri wantErr bool }{ { - name: "add, update and delete kernel boot parameters", - g: GrubCmdline{}, + name: "add, update and delete kernel boot parameters of current partition", + g: GrubCmdline{isCurPartition: true}, args: args{ config: &agent.SysConfig{ Contents: map[string]*agent.KeyInfo{ @@ -220,9 +220,32 @@ menuentry 'B' --class KubeOS --class gnu-linux --class gnu --class os --unrestri pattern: `(?m)^\s+linux\s+\/boot\/vmlinuz\s+root=UUID=[0-1]\s+ro\s+rootfstype=ext4\s+nomodeset\s+oops=panic\s+softlockup_panic=1\s+nmi_watchdog=1\s+rd\.shell=0\s+selinux=0\s+crashkernel=256M\s+panic=5\s+(debug\spci=nomis|pci=nomis\sdebug)$`, wantErr: false, }, + { + name: "add, update and delete kernel boot parameters of next partition", + g: GrubCmdline{isCurPartition: false}, + args: args{ + config: &agent.SysConfig{ + Contents: map[string]*agent.KeyInfo{ + "panic": {Value: "4"}, + "quiet": {Value: "", Operation: "delete"}, + "selinux": {Value: "1", Operation: "delete"}, + "acpi": {Value: "off", Operation: "delete"}, + "debug": {}, + "pci": {Value: "nomis"}, + }, + }, + }, + pattern: `(?m)^\s+linux\s+\/boot\/vmlinuz\s+root=UUID=[0-1]\s+ro\s+rootfstype=ext4\s+nomodeset\s+oops=panic\s+softlockup_panic=1\s+nmi_watchdog=1\s+rd\.shell=0\s+selinux=0\s+crashkernel=256M\s+panic=4\s+(debug\spci=nomis|pci=nomis\sdebug)$`, + wantErr: false, + }, } patchGetGrubPath := gomonkey.ApplyFuncReturn(getGrubCfgPath, grubCfgPath) defer patchGetGrubPath.Reset() + patchGetConfigPartition := gomonkey.ApplyFuncSeq(getConfigPartition, []gomonkey.OutputCell{ + {Values: gomonkey.Params{false, nil}}, + {Values: gomonkey.Params{true, nil}}, + }) + defer patchGetConfigPartition.Reset() for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { g := GrubCmdline{} @@ -236,7 +259,7 @@ menuentry 'B' --class KubeOS --class gnu-linux --class gnu --class os --unrestri re := regexp.MustCompile(tt.pattern) match := re.FindAllStringIndex(string(contents), -1) // it should match partition A and B in total twice - if len(match) != 2 { + if len(match) != 1 { t.Fatalf("expected pattern not found in grub.cfg") } }) @@ -267,7 +290,8 @@ func Test_startConfig(t *testing.T) { configs: []*agent.SysConfig{ {Model: KernelSysctlName.String()}, {Model: KerSysctlPersistName.String()}, - {Model: GrubCmdlineName.String()}, + {Model: GrubCmdlineCurName.String()}, + {Model: GrubCmdlineNextName.String()}, }, }, wantErr: false, diff --git a/cmd/agent/server/values.go b/cmd/agent/server/values.go index bffa376ee2f8d36bc9cb01d91d8732abd9097546..7f16bb970625da25f2673cd8b5a997638d296d6f 100644 --- a/cmd/agent/server/values.go +++ b/cmd/agent/server/values.go @@ -31,8 +31,10 @@ const ( // KerSysctlPersistName is the configuration name of the kernel parameter // set by writing /etc/sysctl.conf or other files KerSysctlPersistName ConfigType = 1 - // GrubCmdlineName is configuration name of cmdline - GrubCmdlineName ConfigType = 2 + // GrubCmdlineCurName is configuration name of current partition's grub cmdline + GrubCmdlineCurName ConfigType = 2 + // GrubCmdlineNextName is configuration name of next partition's grub cmdline + GrubCmdlineNextName ConfigType = 3 ) func (c ConfigType) String() string { @@ -41,8 +43,10 @@ func (c ConfigType) String() string { return "kernel.sysctl" case KerSysctlPersistName: return "kernel.sysctl.persist" - case GrubCmdlineName: - return "grub.cmdline" + case GrubCmdlineCurName: + return "grub.cmdline.current" + case GrubCmdlineNextName: + return "grub.cmdline.next" default: return "unknown" } diff --git a/cmd/operator/controllers/os_controller.go b/cmd/operator/controllers/os_controller.go index 47b068020970c41a929f05b0bd48617ea51e40d7..40610cfaaae8cc8b11b512ec122c5bd3451e793c 100644 --- a/cmd/operator/controllers/os_controller.go +++ b/cmd/operator/controllers/os_controller.go @@ -168,13 +168,17 @@ func assignUpgrade(ctx context.Context, r common.ReadStatusWriter, os upgradev1. } osVersionNode := node.Status.NodeInfo.OSImage if os.Spec.OSVersion != osVersionNode { - count++ - node.Labels[values.LabelUpgrading] = "" var osInstance upgradev1.OSInstance if err = r.Get(ctx, types.NamespacedName{Namespace: nameSpace, Name: node.Name}, &osInstance); err != nil { - log.Error(err, "unable to get osInstance "+node.Name) - return false, err + if err = client.IgnoreNotFound(err); err != nil { + log.Error(err, "failed to get osInstance "+node.Name) + return false, err + } + log.Error(err, "not found osInstance "+node.Name) + continue } + count++ + node.Labels[values.LabelUpgrading] = "" expUpVersion := os.Spec.UpgradeConfigs.Version osiUpVersion := osInstance.Spec.UpgradeConfigs.Version if osiUpVersion != expUpVersion { diff --git a/cmd/operator/controllers/os_controller_test.go b/cmd/operator/controllers/os_controller_test.go index f4fc2b16daa63fd7fc9365c63eefc3dc25afebd6..30a773ca63798f815e786147055729545c3e0dd1 100644 --- a/cmd/operator/controllers/os_controller_test.go +++ b/cmd/operator/controllers/os_controller_test.go @@ -312,4 +312,80 @@ var _ = Describe("OsController", func() { }) }) + Context("When we deploy OS, but there is a node without osinstance", func() { + It("Should not label upgrading and skip that node", func() { + ctx := context.Background() + // create Node + node1Name = "test-node-" + uuid.New().String() + node1 := &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: node1Name, + Namespace: testNamespace, + Labels: map[string]string{ + "beta.kubernetes.io/os": "linux", + }, + }, + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "Node", + }, + Status: v1.NodeStatus{ + NodeInfo: v1.NodeSystemInfo{ + OSImage: "KubeOS v1", + }, + }, + } + err := k8sClient.Create(ctx, node1) + Expect(err).ToNot(HaveOccurred()) + existingNode := &v1.Node{} + Eventually(func() bool { + err := k8sClient.Get(context.Background(), + types.NamespacedName{Name: node1Name, Namespace: testNamespace}, existingNode) + return err == nil + }, timeout, interval).Should(BeTrue()) + + OS := &upgradev1.OS{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "upgrade.openeuler.org/v1alpha1", + Kind: "OS", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: OSName, + Namespace: testNamespace, + }, + Spec: upgradev1.OSSpec{ + OpsType: "upgrade", + MaxUnavailable: 3, + OSVersion: "KubeOS v2", + FlagSafe: true, + MTLS: false, + EvictPodForce: true, + SysConfigs: upgradev1.SysConfigs{ + Configs: []upgradev1.SysConfig{}, + }, + UpgradeConfigs: upgradev1.SysConfigs{Configs: []upgradev1.SysConfig{}}, + }, + } + Expect(k8sClient.Create(ctx, OS)).Should(Succeed()) + + osCRLookupKey := types.NamespacedName{Name: OSName, Namespace: testNamespace} + createdOS := &upgradev1.OS{} + Eventually(func() bool { + err := k8sClient.Get(ctx, osCRLookupKey, createdOS) + return err == nil + }, timeout, interval).Should(BeTrue()) + Expect(createdOS.Spec.OSVersion).Should(Equal("KubeOS v2")) + + time.Sleep(1 * time.Second) // sleep a while to make sure Reconcile finished + existingNode = &v1.Node{} + Eventually(func() bool { + err := k8sClient.Get(context.Background(), + types.NamespacedName{Name: node1Name, Namespace: testNamespace}, existingNode) + return err == nil + }, timeout, interval).Should(BeTrue()) + _, ok := existingNode.Labels[values.LabelUpgrading] + Expect(ok).Should(Equal(false)) + }) + }) + })