From 670c6926e452c00884ba4e0035fceeaf925f1b3a Mon Sep 17 00:00:00 2001 From: chen hui Date: Mon, 19 Jul 2021 08:56:42 +0000 Subject: [PATCH 01/13] TKG-5917 bootstrap script should be executed on the host agent Signed-off-by: chen hui --- agent/cloudinit/cloudinit.go | 80 +++++- agent/cloudinit/cloudinit_test.go | 111 ++++++--- .../cloudinitfakes/fake_ifile_writer.go | 10 +- agent/cloudinit/cmd_runner.go | 11 +- agent/cloudinit/file_writer.go | 114 ++++++++- agent/cloudinit/file_writer_test.go | 86 +++++-- agent/host_agent_test.go | 231 +++++++++++++++++- agent/main.go | 4 +- agent/reconciler/host_reconciler.go | 3 + 9 files changed, 577 insertions(+), 73 deletions(-) diff --git a/agent/cloudinit/cloudinit.go b/agent/cloudinit/cloudinit.go index 828f1d3ed..8b13dc1e9 100644 --- a/agent/cloudinit/cloudinit.go +++ b/agent/cloudinit/cloudinit.go @@ -1,8 +1,13 @@ package cloudinit import ( + "bytes" + "compress/gzip" + "encoding/base64" "fmt" + "io" "path/filepath" + "strings" "github.com/pkg/errors" "sigs.k8s.io/yaml" @@ -19,12 +24,12 @@ type bootstrapConfig struct { } type files struct { - Path string `json:"path,"` - // Encoding string `json:"encoding,omitempty"` - // Owner string `json:"owner,omitempty"` - // Permissions string `json:"permissions,omitempty"` - Content string `json:"content"` - //Append bool `json:"append,"` + Path string `json:"path,"` + Encoding string `json:"encoding,omitempty"` + Owner string `json:"owner,omitempty"` + Permissions string `json:"permissions,omitempty"` + Content string `json:"content"` + Append bool `json:"append,omitempty"` } func (se ScriptExecutor) Execute(bootstrapScript string) error { @@ -40,7 +45,13 @@ func (se ScriptExecutor) Execute(bootstrapScript string) error { return errors.Wrap(err, fmt.Sprintf("Error creating the directory %s", directoryToCreate)) } - err = se.WriteFilesExecutor.WriteToFile(file.Path, file.Content) + encodings := fixEncoding(file.Encoding) + content, err := fixContent(file.Content, encodings) + if err != nil { + return errors.Wrap(err, fmt.Sprintf("error decoding content for %s", file.Path)) + } + + err = se.WriteFilesExecutor.WriteToFile(file.Path, content, file.Permissions, file.Owner, file.Append) if err != nil { return errors.Wrap(err, fmt.Sprintf("Error writing the file %s", file.Path)) } @@ -54,3 +65,58 @@ func (se ScriptExecutor) Execute(bootstrapScript string) error { } return nil } + +func fixEncoding(e string) []string { + e = strings.ToLower(e) + e = strings.TrimSpace(e) + + if e == "gz+base64" || e == "gzip+base64" || e == "gz+b64" || e == "gzip+b64" { + return []string{"application/base64", "application/x-gzip"} + } else if e == "base64" || e == "b64" { + return []string{"application/base64"} + } + + return []string{"text/plain"} +} + +func fixContent(content string, encodings []string) (string, error) { + for _, e := range encodings { + switch e { + case "application/base64": + rByte, err := base64.StdEncoding.DecodeString(content) + if err != nil { + return content, errors.WithStack(err) + } + content = string(rByte) + case "application/x-gzip": + rByte, err := gUnzipData([]byte(content)) + if err != nil { + return content, err + } + content = string(rByte) + case "text/plain": + continue + default: + return content, errors.Errorf("Unknown bootstrap data encoding: %q", content) + } + } + return content, nil +} + +func gUnzipData(data []byte) ([]byte, error) { + var r io.Reader + var err error + b := bytes.NewBuffer(data) + r, err = gzip.NewReader(b) + if err != nil { + return nil, errors.WithStack(err) + } + + var resB bytes.Buffer + _, err = resB.ReadFrom(r) + if err != nil { + return nil, errors.WithStack(err) + } + + return resB.Bytes(), nil +} diff --git a/agent/cloudinit/cloudinit_test.go b/agent/cloudinit/cloudinit_test.go index 9f7f24ed1..e58ce890e 100644 --- a/agent/cloudinit/cloudinit_test.go +++ b/agent/cloudinit/cloudinit_test.go @@ -1,30 +1,41 @@ package cloudinit_test import ( + "encoding/base64" "errors" + "fmt" + "io/ioutil" + "os" + "path" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" - "github.com/vmware-tanzu/cluster-api-provider-byoh/agent/cloudinit" "github.com/vmware-tanzu/cluster-api-provider-byoh/agent/cloudinit/cloudinitfakes" ) -var someBootstrapSecret = ` -write_files: -- path: /tmp/file1.txt - content: some-content -runCmd: -- echo 'some run command' -` - var _ = Describe("Cloudinit", func() { + var ( + workDir string + err error + ) + + BeforeEach(func() { + workDir, err = ioutil.TempDir("", "cloudinit_ut") + Expect(err).ToNot(HaveOccurred()) + }) + + AfterEach(func() { + err := os.RemoveAll(workDir) + Expect(err).ToNot(HaveOccurred()) + }) + Context("Testing write_files and runCmd directives of cloudinit", func() { var ( - fakeFileWriter *cloudinitfakes.FakeIFileWriter - fakeCmdExecutor *cloudinitfakes.FakeICmdRunner - scriptExecutor cloudinit.ScriptExecutor - err error + fakeFileWriter *cloudinitfakes.FakeIFileWriter + fakeCmdExecutor *cloudinitfakes.FakeICmdRunner + scriptExecutor cloudinit.ScriptExecutor + defaultBootstrapSecret string ) BeforeEach(func() { @@ -34,14 +45,34 @@ var _ = Describe("Cloudinit", func() { WriteFilesExecutor: fakeFileWriter, RunCmdExecutor: fakeCmdExecutor, } + + defaultBootstrapSecret = fmt.Sprintf(`write_files: +- path: %s/defaultFile.txt + content: some-content +runCmd: +- echo 'some run command'`, workDir) + }) + + AfterEach(func() { + err := os.RemoveAll(workDir) + Expect(err).ToNot(HaveOccurred()) }) It("should write files successfully", func() { - bootstrapSecretUnencoded := `write_files: -- path: /tmp/a/file1.txt - content: some-content -- path: /tmp/b/file2.txt - content: whatever` + fileDir1 := path.Join(workDir, "dir1") + fileName1 := path.Join(fileDir1, "file1.txt") + fileContent1 := "some-content-1" + + fileDir2 := path.Join(workDir, "dir2") + fileName2 := path.Join(fileDir2, "file2.txt") + fileContent2 := "some-content-2" + + bootstrapSecretUnencoded := fmt.Sprintf(`write_files: +- path: %s + content: %s +- path: %s + content: %s`, fileName1, fileContent1, fileName2, fileContent2) + err = scriptExecutor.Execute(bootstrapSecretUnencoded) Expect(err).ToNot(HaveOccurred()) @@ -49,16 +80,39 @@ var _ = Describe("Cloudinit", func() { Expect(fakeFileWriter.WriteToFileCallCount()).To(Equal(2)) dirNameForFirstFile := fakeFileWriter.MkdirIfNotExistsArgsForCall(0) - Expect(dirNameForFirstFile).To(Equal("/tmp/a")) + Expect(dirNameForFirstFile).To(Equal(fileDir1)) firstFileName, firstFileContents := fakeFileWriter.WriteToFileArgsForCall(0) - Expect(firstFileName).To(Equal("/tmp/a/file1.txt")) - Expect(firstFileContents).To(Equal("some-content")) + Expect(firstFileName).To(Equal(fileName1)) + Expect(firstFileContents).To(Equal(fileContent1)) dirNameForSecondFile := fakeFileWriter.MkdirIfNotExistsArgsForCall(1) - Expect(dirNameForSecondFile).To(Equal("/tmp/b")) + Expect(dirNameForSecondFile).To(Equal(fileDir2)) secondFileName, secondFileContents := fakeFileWriter.WriteToFileArgsForCall(1) - Expect(secondFileName).To(Equal("/tmp/b/file2.txt")) - Expect(secondFileContents).To(Equal("whatever")) + Expect(secondFileName).To(Equal(fileName2)) + Expect(secondFileContents).To(Equal(fileContent2)) + }) + + It("could recognize owner, permissions, and append attributes", func() { + + fileDir := path.Join(workDir, "dir") + fileName := path.Join(fileDir, "file.txt") + fileContent := "some-content-append" + fileBase64Content := base64.StdEncoding.EncodeToString([]byte(fileContent)) + user := "root" + group := "root" + permissions := "0777" + encoding := "base64" + + bootstrapSecretUnencoded := fmt.Sprintf(`write_files: +- path: %s + content: %s + owner: %s:%s + permissions: '%s' + append: true + encoding: %s`, fileName, fileBase64Content, user, group, permissions, encoding) + + err = scriptExecutor.Execute(bootstrapSecretUnencoded) + Expect(err).ToNot(HaveOccurred()) }) It("should error out when an invalid yaml is passed", func() { @@ -71,7 +125,7 @@ var _ = Describe("Cloudinit", func() { It("should error out when there is not enough permission to mkdir", func() { fakeFileWriter.MkdirIfNotExistsReturns(errors.New("not enough permissions")) - err := scriptExecutor.Execute(someBootstrapSecret) + err := scriptExecutor.Execute(defaultBootstrapSecret) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("not enough permissions")) @@ -82,14 +136,14 @@ var _ = Describe("Cloudinit", func() { It("should error out write to file failes", func() { fakeFileWriter.WriteToFileReturns(errors.New("cannot write to file")) - err := scriptExecutor.Execute(someBootstrapSecret) + err := scriptExecutor.Execute(defaultBootstrapSecret) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("cannot write to file")) }) It("run the command given in the runCmd directive", func() { - err := scriptExecutor.Execute(someBootstrapSecret) + err := scriptExecutor.Execute(defaultBootstrapSecret) Expect(err).ToNot(HaveOccurred()) Expect(fakeCmdExecutor.RunCmdCallCount()).To(Equal(1)) @@ -108,9 +162,8 @@ var _ = Describe("Cloudinit", func() { }) It("should error out when command execution fails", func() { - fakeCmdExecutor.RunCmdReturns(errors.New("command execution failed")) - err := scriptExecutor.Execute(someBootstrapSecret) + err := scriptExecutor.Execute(defaultBootstrapSecret) Expect(err).To(HaveOccurred()) Expect(fakeCmdExecutor.RunCmdCallCount()).To(Equal(1)) diff --git a/agent/cloudinit/cloudinitfakes/fake_ifile_writer.go b/agent/cloudinit/cloudinitfakes/fake_ifile_writer.go index 1779f845b..ec823f5a4 100644 --- a/agent/cloudinit/cloudinitfakes/fake_ifile_writer.go +++ b/agent/cloudinit/cloudinitfakes/fake_ifile_writer.go @@ -24,6 +24,9 @@ type FakeIFileWriter struct { writeToFileArgsForCall []struct { arg1 string arg2 string + arg3 string + arg4 string + arg5 bool } writeToFileReturns struct { result1 error @@ -96,13 +99,16 @@ func (fake *FakeIFileWriter) MkdirIfNotExistsReturnsOnCall(i int, result1 error) }{result1} } -func (fake *FakeIFileWriter) WriteToFile(arg1 string, arg2 string) error { +func (fake *FakeIFileWriter) WriteToFile(arg1 string, arg2 string, arg3 string, arg4 string, arg5 bool) error { fake.writeToFileMutex.Lock() ret, specificReturn := fake.writeToFileReturnsOnCall[len(fake.writeToFileArgsForCall)] fake.writeToFileArgsForCall = append(fake.writeToFileArgsForCall, struct { arg1 string arg2 string - }{arg1, arg2}) + arg3 string + arg4 string + arg5 bool + }{arg1, arg2, arg3, arg4, arg5}) stub := fake.WriteToFileStub fakeReturns := fake.writeToFileReturns fake.recordInvocation("WriteToFile", []interface{}{arg1, arg2}) diff --git a/agent/cloudinit/cmd_runner.go b/agent/cloudinit/cmd_runner.go index b827b7890..2f6760f6f 100644 --- a/agent/cloudinit/cmd_runner.go +++ b/agent/cloudinit/cmd_runner.go @@ -15,13 +15,18 @@ type CmdRunner struct { } func (r CmdRunner) RunCmd(cmd string) error { + subStrs := []string{"kubeadm init", "kubeadm join"} - if strings.Contains(cmd, "kubeadm") { - cmd = "kubeadm join --config /run/kubeadm/kubeadm-join-config.yaml --ignore-preflight-errors=all && echo success > /run/cluster-api/bootstrap-success.complete" + for _, subStr := range subStrs { + if strings.Contains(cmd, subStr) { + index := strings.Index(cmd, subStr) + index += len(subStr) + newCmd := cmd[:index] + " --ignore-preflight-errors=all " + cmd[index:] + cmd = newCmd + } } command := exec.Command("/bin/sh", "-c", cmd) output, err := command.Output() fmt.Println(string(output)) return err - } diff --git a/agent/cloudinit/file_writer.go b/agent/cloudinit/file_writer.go index ee2092d87..ee8f2926d 100644 --- a/agent/cloudinit/file_writer.go +++ b/agent/cloudinit/file_writer.go @@ -1,7 +1,16 @@ package cloudinit import ( + "fmt" + "io/fs" "os" + "os/exec" + "os/user" + "strconv" + "strings" + "syscall" + + "github.com/pkg/errors" ) //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 -generate @@ -9,7 +18,7 @@ import ( //counterfeiter:generate . IFileWriter type IFileWriter interface { MkdirIfNotExists(string) error - WriteToFile(string, string) error + WriteToFile(string, string, string, string, bool) error } type FileWriter struct { @@ -29,13 +38,110 @@ func (w FileWriter) MkdirIfNotExists(dirName string) error { } -func (w FileWriter) WriteToFile(fileName string, fileContent string) error { - f, err := os.Create(fileName) +func (w FileWriter) WriteToFile(fileName string, fileContent string, filePermission string, fileOwner string, append bool) error { + initPermission := fs.FileMode(0644) + if stats, err := os.Stat(fileName); os.IsExist(err) { + initPermission = stats.Mode() + } + + flag := os.O_WRONLY | os.O_CREATE + if append { + flag |= os.O_APPEND + } + + f, err := os.OpenFile(fileName, flag, initPermission) if err != nil { return err } defer f.Close() _, err = f.WriteString(fileContent) - return err + if err != nil { + return err + } + + u, err := user.Current() + if err != nil { + return err + } + if len(filePermission) > 0 { + isRootOrFileOwner := false + if u.Username != "root" { + stats, err := os.Stat(fileName) + if err != nil { + return err + } + stat := stats.Sys().(*syscall.Stat_t) + if u.Uid == strconv.FormatUint(uint64(stat.Uid), 10) && u.Gid == strconv.FormatUint(uint64(stat.Gid), 10) { + isRootOrFileOwner = true + } + } else { + isRootOrFileOwner = true + } + + //Fetch permission information + if isRootOrFileOwner { + //Make sure agent run as root or file owner + fileMode, err := strconv.ParseUint(filePermission, 8, 32) + if err != nil { + return errors.Wrap(err, fmt.Sprintf("Error parse the file permission %s", filePermission)) + } + + err = f.Chmod(fs.FileMode(fileMode)) + if err != nil { + return err + } + } else { + //Make sure current user is sudoer, and there is "sudo" and "chmod" command in system + cmd := fmt.Sprintf("sudo chmod %s %s", filePermission, fileName) + command := exec.Command("/bin/sh", "-c", cmd) + _, err := command.Output() + if err != nil { + return err + } + } + } + + if len(fileOwner) > 0 { + + //Fetch owner information + if u.Username == "root" { + //only root can do this + owner := strings.Split(fileOwner, ":") + if len(owner) != 2 { + return errors.Wrap(err, fmt.Sprintf("Invalid owner format '%s'", fileOwner)) + } + + userInfo, err := user.Lookup(owner[0]) + if err != nil { + return errors.Wrap(err, fmt.Sprintf("Error Lookup user %s", owner[0])) + } + + uid, err := strconv.ParseUint(userInfo.Uid, 10, 32) + if err != nil { + return errors.Wrap(err, fmt.Sprintf("Error convert uid %s", userInfo.Uid)) + } + + gid, err := strconv.ParseUint(userInfo.Gid, 10, 32) + if err != nil { + return errors.Wrap(err, fmt.Sprintf("Error convert gid %s", userInfo.Gid)) + } + + err = f.Chown(int(uid), int(gid)) + if err != nil { + return err + } + } else { + //Make sure current user is sudoer, and there is "sudo" and "chown" command in system + cmd := fmt.Sprintf("sudo chown %s %s", fileOwner, fileName) + command := exec.Command("/bin/sh", "-c", cmd) + _, err := command.Output() + if err != nil { + return err + } + } + } + + return nil + } diff --git a/agent/cloudinit/file_writer_test.go b/agent/cloudinit/file_writer_test.go index 73caeaece..b932d5372 100644 --- a/agent/cloudinit/file_writer_test.go +++ b/agent/cloudinit/file_writer_test.go @@ -1,53 +1,111 @@ package cloudinit import ( + "io/fs" "io/ioutil" "os" + "os/user" "path" + "strconv" + "syscall" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" ) var _ = Describe("FileWriter", func() { + var ( - dir string - err error + workDir string + err error ) BeforeEach(func() { - dir, err = ioutil.TempDir("", "cloudinit") + workDir, err = ioutil.TempDir("", "file_writer_ut") Expect(err).ToNot(HaveOccurred()) }) - It("Should create a directory if it does not exists", func() { - err := FileWriter{}.MkdirIfNotExists(path.Join(dir, "test")) + AfterEach(func() { + err := os.RemoveAll(workDir) + Expect(err).ToNot(HaveOccurred()) + }) + It("Should create a directory if it does not exists", func() { + err := FileWriter{}.MkdirIfNotExists(workDir) Expect(err).ToNot(HaveOccurred()) }) It("Should not create a directory if it already exists", func() { - FileWriter{}.MkdirIfNotExists(path.Join(dir, "test")) - - err = FileWriter{}.MkdirIfNotExists(path.Join(dir, "test")) + FileWriter{}.MkdirIfNotExists(workDir) + Expect(err).ToNot(HaveOccurred()) + err = FileWriter{}.MkdirIfNotExists(workDir) Expect(err).ToNot(HaveOccurred()) }) It("Should create and write to file", func() { - FileWriter{}.MkdirIfNotExists(path.Join(dir, "test")) + fileName := path.Join(workDir, "file1.txt") + fileContent := "some-content" - err := FileWriter{}.WriteToFile(path.Join(dir, "test", "file1.txt"), "some-content") + err := FileWriter{}.MkdirIfNotExists(workDir) + Expect(err).ToNot(HaveOccurred()) + err = FileWriter{}.WriteToFile(fileName, fileContent, "", "", false) Expect(err).NotTo(HaveOccurred()) - buffer, err := ioutil.ReadFile(path.Join(dir, "test", "file1.txt")) + + buffer, err := ioutil.ReadFile(fileName) Expect(err).NotTo(HaveOccurred()) - Expect(string(buffer)).To(Equal("some-content")) + Expect(string(buffer)).To(Equal(fileContent)) }) - AfterEach(func() { - err := os.RemoveAll(dir) + It("Should create and write to file with correct attributes", func() { + + fileName := path.Join(workDir, "file2.txt") + userName := "root" + groupName := "root" + fileContent := "some-content" + filePermission := 0777 + + err := FileWriter{}.MkdirIfNotExists(workDir) Expect(err).ToNot(HaveOccurred()) + + err = FileWriter{}.WriteToFile(fileName, fileContent, strconv.FormatInt(int64(filePermission), 8), userName+":"+groupName, false) + Expect(err).NotTo(HaveOccurred()) + + stats, err := os.Stat(fileName) + Expect(err).NotTo(HaveOccurred()) + Expect(stats.Mode()).To(Equal(fs.FileMode(filePermission))) + + userInfo, err := user.Lookup(userName) + Expect(err).NotTo(HaveOccurred()) + uid, err := strconv.ParseUint(userInfo.Uid, 10, 32) + Expect(err).NotTo(HaveOccurred()) + gid, err := strconv.ParseUint(userInfo.Gid, 10, 32) + Expect(err).NotTo(HaveOccurred()) + + stat := stats.Sys().(*syscall.Stat_t) + Expect(stat.Uid).To(Equal(uint32(uid))) + Expect(stat.Gid).To(Equal(uint32(gid))) + }) + + It("Should append content to file when append mode is enabled", func() { + fileName := path.Join(workDir, "file3.txt") + fileOriginContent := "some-content-1" + fileAppendContent := "some-content-2" + + err := FileWriter{}.MkdirIfNotExists(workDir) + Expect(err).NotTo(HaveOccurred()) + + err = ioutil.WriteFile(fileName, []byte(fileOriginContent), 0644) + Expect(err).NotTo(HaveOccurred()) + + err = FileWriter{}.WriteToFile(fileName, fileAppendContent, "", "", true) + Expect(err).NotTo(HaveOccurred()) + + buffer, err := ioutil.ReadFile(fileName) + Expect(err).NotTo(HaveOccurred()) + Expect(string(buffer)).To(Equal(fileOriginContent + fileAppendContent)) + }) }) diff --git a/agent/host_agent_test.go b/agent/host_agent_test.go index 4a4631658..cc38f5488 100644 --- a/agent/host_agent_test.go +++ b/agent/host_agent_test.go @@ -1,12 +1,19 @@ package main import ( + "bytes" + "compress/gzip" "context" + "encoding/base64" "fmt" + "io/fs" "io/ioutil" "os" "os/exec" + "os/user" "path" + "strconv" + "syscall" "time" . "github.com/onsi/ginkgo" @@ -70,7 +77,7 @@ var _ = Describe("Agent", func() { ns *corev1.Namespace session *gexec.Session err error - dir string + workDir string hostName string ) @@ -86,14 +93,14 @@ var _ = Describe("Agent", func() { session, err = gexec.Start(command, GinkgoWriter, GinkgoWriter) Expect(err).ToNot(HaveOccurred()) - dir, err = ioutil.TempDir("", "cloudinit") + workDir, err = ioutil.TempDir("", "host-agent-ut") Expect(err).ToNot(HaveOccurred()) }) AfterEach(func() { err = k8sClient.Delete(context.TODO(), ns) Expect(err).ToNot(HaveOccurred()) - os.RemoveAll(dir) + os.RemoveAll(workDir) session.Terminate().Wait() }) @@ -110,6 +117,15 @@ var _ = Describe("Agent", func() { }) It("should bootstrap the node when MachineRef is set", func() { + + bootstrapSecretName := "bootstrap-secret-1" + machineName := "test-machine-1" + byoMachineName := "test-byomachine-1" + + fileName := path.Join(workDir, "file.txt") + fileOriginContent := "some-content-1" + fileNewContent := " run cmd" + byoHost := &infrastructurev1alpha4.ByoHost{} byoHostLookupKey := types.NamespacedName{Name: hostName, Namespace: ns.Name} Eventually(func() bool { @@ -117,21 +133,19 @@ var _ = Describe("Agent", func() { return err == nil }).ShouldNot(BeFalse()) - fileToCreate := path.Join(dir, "test-directory", "test-file.txt") - bootstrapSecretUnencoded := fmt.Sprintf(`write_files: - path: %s - content: expected-content + content: %s runCmd: -- echo -n ' run cmd' >> %s`, fileToCreate, fileToCreate) +- echo -n '%s' >> %s`, fileName, fileOriginContent, fileNewContent, fileName) - secret, err := createSecret("bootstrap-secret-1", bootstrapSecretUnencoded, ns.Name) + secret, err := createSecret(bootstrapSecretName, bootstrapSecretUnencoded, ns.Name) Expect(err).ToNot(HaveOccurred()) - machine, err := createMachine(&secret.Name, "test-machine", ns.Name) + machine, err := createMachine(&secret.Name, machineName, ns.Name) Expect(err).ToNot(HaveOccurred()) - byoMachine, err := createByoMachine("test-byomachine", ns.Name, machine) + byoMachine, err := createByoMachine(byoMachineName, ns.Name, machine) Expect(err).ToNot(HaveOccurred()) helper, err := patch.NewHelper(byoHost, k8sClient) @@ -162,18 +176,211 @@ runCmd: }).Should(Equal(corev1.ConditionTrue)) Eventually(func() string { - buffer, err := ioutil.ReadFile(fileToCreate) + buffer, err := ioutil.ReadFile(fileName) if err != nil { return "" } return string(buffer) - }).Should(Equal("expected-content run cmd")) + }).Should(Equal(fileOriginContent + fileNewContent)) + + }) + + It("check if file created by bootstrap script in correct attributes", func() { + bootstrapSecretName := "bootstrap-secret-2" + machineName := "test-machine-2" + byoMachineName := "test-byomachine-2" + fileName := path.Join(workDir, "file-2.txt") + fileOriginContent := "some-content-2" + fileAppendContent := "some-content-append" + filePermission := 0777 + userName := "root" + groupName := "root" + isAppend := true + + //Init file + err = ioutil.WriteFile(fileName, []byte(fileOriginContent), 0644) + Expect(err).NotTo(HaveOccurred()) + + bootstrapSecretUnencoded := fmt.Sprintf(`write_files: +- path: %s + owner: %s:%s + permissions: '%s' + content: %s + append: %v`, fileName, userName, groupName, strconv.FormatInt(int64(filePermission), 8), fileAppendContent, isAppend) + + byoHost := &infrastructurev1alpha4.ByoHost{} + byoHostLookupKey := types.NamespacedName{Name: hostName, Namespace: ns.Name} + Eventually(func() bool { + err = k8sClient.Get(context.TODO(), byoHostLookupKey, byoHost) + return err == nil + }).ShouldNot(BeFalse()) + + secret, err := createSecret(bootstrapSecretName, bootstrapSecretUnencoded, ns.Name) + Expect(err).ToNot(HaveOccurred()) + + machine, err := createMachine(&secret.Name, machineName, ns.Name) + Expect(err).ToNot(HaveOccurred()) + + byoMachine, err := createByoMachine(byoMachineName, ns.Name, machine) + Expect(err).ToNot(HaveOccurred()) + + helper, err := patch.NewHelper(byoHost, k8sClient) + Expect(err).ToNot(HaveOccurred()) + + byoHost.Status.MachineRef = &corev1.ObjectReference{ + Kind: "ByoMachine", + Namespace: ns.Name, + Name: byoMachine.Name, + UID: byoMachine.UID, + APIVersion: byoHost.APIVersion, + } + err = helper.Patch(context.TODO(), byoHost) + Expect(err).ToNot(HaveOccurred()) + + //check file content + Eventually(func() string { + buffer, err := ioutil.ReadFile(fileName) + if err != nil { + return "" + } + return string(buffer) + }).Should(Equal(fileOriginContent + fileAppendContent)) + + //check file permission + Eventually(func() bool { + stats, err := os.Stat(fileName) + if err == nil && stats.Mode() == fs.FileMode(filePermission) { + return true + } + return false + }).Should(BeTrue()) + + //check file owner + Eventually(func() bool { + stats, err := os.Stat(fileName) + if err != nil { + return false + } + stat := stats.Sys().(*syscall.Stat_t) + + userInfo, err := user.Lookup(userName) + if err != nil { + return false + } + + uid, err := strconv.ParseUint(userInfo.Uid, 10, 32) + if err != nil { + return false + } + + gid, err := strconv.ParseUint(userInfo.Gid, 10, 32) + if err != nil { + return false + } + + if stat.Uid == uint32(uid) && stat.Gid == uint32(gid) { + return true + } + return false + + }).Should(BeTrue()) + + }) + + It("check if file created by bootstrap script in correct encoding way", func() { + bootstrapSecretName := "bootstrap-secret-3" + machineName := "test-machine-3" + byoMachineName := "test-byomachine-3" + + fileName1 := path.Join(workDir, "file-3-1.txt") + fileContent1 := "some-content-3-1" + fileBase64Content1 := base64.StdEncoding.EncodeToString([]byte(fileContent1)) + + fileName2 := path.Join(workDir, "file-3-2.txt") + fileContent2 := "some-content-3-2" + fileGzipContent2, err := gZipData([]byte(fileContent2)) + Expect(err).ToNot(HaveOccurred()) + fileGzipBase64Content2 := base64.StdEncoding.EncodeToString(fileGzipContent2) + + bootstrapSecretUnencoded := fmt.Sprintf(`write_files: +- path: %s + content: %s + encoding: base64 +- path: %s + encoding: gzip+base64 + content: %s`, fileName1, fileBase64Content1, fileName2, fileGzipBase64Content2) + + byoHost := &infrastructurev1alpha4.ByoHost{} + byoHostLookupKey := types.NamespacedName{Name: hostName, Namespace: ns.Name} + Eventually(func() bool { + err = k8sClient.Get(context.TODO(), byoHostLookupKey, byoHost) + return err == nil + }).ShouldNot(BeFalse()) + + secret, err := createSecret(bootstrapSecretName, bootstrapSecretUnencoded, ns.Name) + Expect(err).ToNot(HaveOccurred()) + + machine, err := createMachine(&secret.Name, machineName, ns.Name) + Expect(err).ToNot(HaveOccurred()) + + byoMachine, err := createByoMachine(byoMachineName, ns.Name, machine) + Expect(err).ToNot(HaveOccurred()) + + helper, err := patch.NewHelper(byoHost, k8sClient) + Expect(err).ToNot(HaveOccurred()) + + byoHost.Status.MachineRef = &corev1.ObjectReference{ + Kind: "ByoMachine", + Namespace: ns.Name, + Name: byoMachine.Name, + UID: byoMachine.UID, + APIVersion: byoHost.APIVersion, + } + err = helper.Patch(context.TODO(), byoHost) + Expect(err).ToNot(HaveOccurred()) + + //bas64 + Eventually(func() string { + buffer, err := ioutil.ReadFile(fileName1) + if err != nil { + return "" + } + return string(buffer) + }).Should(Equal(fileContent1)) + + //gzip+base64 + Eventually(func() string { + buffer, err := ioutil.ReadFile(fileName2) + if err != nil { + return "" + } + return string(buffer) + }).Should(Equal(fileContent2)) }) }) }) +func gZipData(data []byte) ([]byte, error) { + var b bytes.Buffer + gz := gzip.NewWriter(&b) + + if _, err := gz.Write(data); err != nil { + return nil, err + } + + if err := gz.Flush(); err != nil { + return nil, err + } + + if err := gz.Close(); err != nil { + return nil, err + } + + return b.Bytes(), nil +} + func RandStr(length int) string { str := "0123456789abcdefghijklmnopqrstuvwxyz" bytes := []byte(str) diff --git a/agent/main.go b/agent/main.go index 6819a6337..a428cf437 100644 --- a/agent/main.go +++ b/agent/main.go @@ -60,7 +60,7 @@ func main() { Namespace: namespace, }) if err != nil { - klog.Fatal(err, "unable to start manager") + klog.Fatal(errors.Wrap(err, "unable to start manager")) } hostReconciler := reconciler.HostReconciler{Client: k8sClient} @@ -69,6 +69,6 @@ func main() { Complete(hostReconciler) if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil { - klog.Fatal(err, "problem running manager") + klog.Fatal(errors.Wrap(err, "problem running manager")) } } diff --git a/agent/reconciler/host_reconciler.go b/agent/reconciler/host_reconciler.go index 522a26411..6e1ee900d 100644 --- a/agent/reconciler/host_reconciler.go +++ b/agent/reconciler/host_reconciler.go @@ -27,6 +27,7 @@ func (r HostReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.R err := r.Client.Get(ctx, req.NamespacedName, byoHost) if err != nil { klog.Fatal(err) + return ctrl.Result{}, err } if byoHost.Status.MachineRef == nil { @@ -44,6 +45,7 @@ func (r HostReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.R RunCmdExecutor: cloudinit.CmdRunner{}}.Execute(bootstrapScript) if err != nil { klog.Fatal(err) + return ctrl.Result{}, err } helper, err := patch.NewHelper(byoHost, r.Client) @@ -54,6 +56,7 @@ func (r HostReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.R err = helper.Patch(ctx, byoHost) if err != nil { klog.Fatal(err) + return ctrl.Result{}, err } return ctrl.Result{}, nil From 96bff17544f65794078c7f23a97e1303c45b0083 Mon Sep 17 00:00:00 2001 From: chen hui Date: Wed, 21 Jul 2021 01:59:30 +0000 Subject: [PATCH 02/13] comment from anusha Signed-off-by: chen hui --- agent/cloudinit/cloudinit.go | 14 +- agent/cloudinit/cloudinit_test.go | 45 ++--- .../cloudinitfakes/fake_ifile_writer.go | 28 +-- agent/cloudinit/file_writer.go | 30 +-- agent/cloudinit/file_writer_test.go | 52 +++-- agent/host_agent_test.go | 184 ++++++------------ 6 files changed, 139 insertions(+), 214 deletions(-) diff --git a/agent/cloudinit/cloudinit.go b/agent/cloudinit/cloudinit.go index 8b13dc1e9..020728e0e 100644 --- a/agent/cloudinit/cloudinit.go +++ b/agent/cloudinit/cloudinit.go @@ -19,11 +19,11 @@ type ScriptExecutor struct { } type bootstrapConfig struct { - FilesToWrite []files `json:"write_files"` + FilesToWrite []Files `json:"write_files"` CommandsToExecute []string `json:"runCmd"` } -type files struct { +type Files struct { Path string `json:"path,"` Encoding string `json:"encoding,omitempty"` Owner string `json:"owner,omitempty"` @@ -46,12 +46,11 @@ func (se ScriptExecutor) Execute(bootstrapScript string) error { } encodings := fixEncoding(file.Encoding) - content, err := fixContent(file.Content, encodings) + file.Content, err = fixContent(file.Content, encodings) if err != nil { return errors.Wrap(err, fmt.Sprintf("error decoding content for %s", file.Path)) } - - err = se.WriteFilesExecutor.WriteToFile(file.Path, content, file.Permissions, file.Owner, file.Append) + err = se.WriteFilesExecutor.WriteToFile(file) if err != nil { return errors.Wrap(err, fmt.Sprintf("Error writing the file %s", file.Path)) } @@ -70,9 +69,10 @@ func fixEncoding(e string) []string { e = strings.ToLower(e) e = strings.TrimSpace(e) - if e == "gz+base64" || e == "gzip+base64" || e == "gz+b64" || e == "gzip+b64" { + switch e { + case "gz+base64", "gzip+base64", "gz+b64", "gzip+b64": return []string{"application/base64", "application/x-gzip"} - } else if e == "base64" || e == "b64" { + case "base64", "b64": return []string{"application/base64"} } diff --git a/agent/cloudinit/cloudinit_test.go b/agent/cloudinit/cloudinit_test.go index e58ce890e..0bd277a03 100644 --- a/agent/cloudinit/cloudinit_test.go +++ b/agent/cloudinit/cloudinit_test.go @@ -66,12 +66,21 @@ runCmd: fileDir2 := path.Join(workDir, "dir2") fileName2 := path.Join(fileDir2, "file2.txt") fileContent2 := "some-content-2" + fileBase64Content := base64.StdEncoding.EncodeToString([]byte(fileContent2)) + user := "root" + group := "root" + permissions := "0777" + encoding := "base64" bootstrapSecretUnencoded := fmt.Sprintf(`write_files: - path: %s content: %s - path: %s - content: %s`, fileName1, fileContent1, fileName2, fileContent2) + content: %s + owner: %s:%s + permissions: '%s' + append: true + encoding: %s`, fileName1, fileContent1, fileName2, fileBase64Content, user, group, permissions, encoding) err = scriptExecutor.Execute(bootstrapSecretUnencoded) Expect(err).ToNot(HaveOccurred()) @@ -81,38 +90,16 @@ runCmd: dirNameForFirstFile := fakeFileWriter.MkdirIfNotExistsArgsForCall(0) Expect(dirNameForFirstFile).To(Equal(fileDir1)) - firstFileName, firstFileContents := fakeFileWriter.WriteToFileArgsForCall(0) - Expect(firstFileName).To(Equal(fileName1)) - Expect(firstFileContents).To(Equal(fileContent1)) + firstFile := fakeFileWriter.WriteToFileArgsForCall(0) + Expect(firstFile.Path).To(Equal(fileName1)) + Expect(firstFile.Content).To(Equal(fileContent1)) dirNameForSecondFile := fakeFileWriter.MkdirIfNotExistsArgsForCall(1) Expect(dirNameForSecondFile).To(Equal(fileDir2)) - secondFileName, secondFileContents := fakeFileWriter.WriteToFileArgsForCall(1) - Expect(secondFileName).To(Equal(fileName2)) - Expect(secondFileContents).To(Equal(fileContent2)) - }) - - It("could recognize owner, permissions, and append attributes", func() { - - fileDir := path.Join(workDir, "dir") - fileName := path.Join(fileDir, "file.txt") - fileContent := "some-content-append" - fileBase64Content := base64.StdEncoding.EncodeToString([]byte(fileContent)) - user := "root" - group := "root" - permissions := "0777" - encoding := "base64" - - bootstrapSecretUnencoded := fmt.Sprintf(`write_files: -- path: %s - content: %s - owner: %s:%s - permissions: '%s' - append: true - encoding: %s`, fileName, fileBase64Content, user, group, permissions, encoding) + secondFile := fakeFileWriter.WriteToFileArgsForCall(1) + Expect(secondFile.Path).To(Equal(fileName2)) + Expect(secondFile.Content).To(Equal(fileContent2)) - err = scriptExecutor.Execute(bootstrapSecretUnencoded) - Expect(err).ToNot(HaveOccurred()) }) It("should error out when an invalid yaml is passed", func() { diff --git a/agent/cloudinit/cloudinitfakes/fake_ifile_writer.go b/agent/cloudinit/cloudinitfakes/fake_ifile_writer.go index ec823f5a4..40ec6a068 100644 --- a/agent/cloudinit/cloudinitfakes/fake_ifile_writer.go +++ b/agent/cloudinit/cloudinitfakes/fake_ifile_writer.go @@ -19,14 +19,10 @@ type FakeIFileWriter struct { mkdirIfNotExistsReturnsOnCall map[int]struct { result1 error } - WriteToFileStub func(string, string) error + WriteToFileStub func(cloudinit.Files) error writeToFileMutex sync.RWMutex writeToFileArgsForCall []struct { - arg1 string - arg2 string - arg3 string - arg4 string - arg5 bool + arg1 cloudinit.Files } writeToFileReturns struct { result1 error @@ -99,22 +95,18 @@ func (fake *FakeIFileWriter) MkdirIfNotExistsReturnsOnCall(i int, result1 error) }{result1} } -func (fake *FakeIFileWriter) WriteToFile(arg1 string, arg2 string, arg3 string, arg4 string, arg5 bool) error { +func (fake *FakeIFileWriter) WriteToFile(arg1 cloudinit.Files) error { fake.writeToFileMutex.Lock() ret, specificReturn := fake.writeToFileReturnsOnCall[len(fake.writeToFileArgsForCall)] fake.writeToFileArgsForCall = append(fake.writeToFileArgsForCall, struct { - arg1 string - arg2 string - arg3 string - arg4 string - arg5 bool - }{arg1, arg2, arg3, arg4, arg5}) + arg1 cloudinit.Files + }{arg1}) stub := fake.WriteToFileStub fakeReturns := fake.writeToFileReturns - fake.recordInvocation("WriteToFile", []interface{}{arg1, arg2}) + fake.recordInvocation("WriteToFile", []interface{}{arg1}) fake.writeToFileMutex.Unlock() if stub != nil { - return stub(arg1, arg2) + return stub(arg1) } if specificReturn { return ret.result1 @@ -128,17 +120,17 @@ func (fake *FakeIFileWriter) WriteToFileCallCount() int { return len(fake.writeToFileArgsForCall) } -func (fake *FakeIFileWriter) WriteToFileCalls(stub func(string, string) error) { +func (fake *FakeIFileWriter) WriteToFileCalls(stub func(cloudinit.Files) error) { fake.writeToFileMutex.Lock() defer fake.writeToFileMutex.Unlock() fake.WriteToFileStub = stub } -func (fake *FakeIFileWriter) WriteToFileArgsForCall(i int) (string, string) { +func (fake *FakeIFileWriter) WriteToFileArgsForCall(i int) cloudinit.Files { fake.writeToFileMutex.RLock() defer fake.writeToFileMutex.RUnlock() argsForCall := fake.writeToFileArgsForCall[i] - return argsForCall.arg1, argsForCall.arg2 + return argsForCall.arg1 } func (fake *FakeIFileWriter) WriteToFileReturns(result1 error) { diff --git a/agent/cloudinit/file_writer.go b/agent/cloudinit/file_writer.go index ee8f2926d..8b781221b 100644 --- a/agent/cloudinit/file_writer.go +++ b/agent/cloudinit/file_writer.go @@ -18,7 +18,7 @@ import ( //counterfeiter:generate . IFileWriter type IFileWriter interface { MkdirIfNotExists(string) error - WriteToFile(string, string, string, string, bool) error + WriteToFile(Files) error } type FileWriter struct { @@ -38,24 +38,24 @@ func (w FileWriter) MkdirIfNotExists(dirName string) error { } -func (w FileWriter) WriteToFile(fileName string, fileContent string, filePermission string, fileOwner string, append bool) error { +func (w FileWriter) WriteToFile(file Files) error { initPermission := fs.FileMode(0644) - if stats, err := os.Stat(fileName); os.IsExist(err) { + if stats, err := os.Stat(file.Path); os.IsExist(err) { initPermission = stats.Mode() } flag := os.O_WRONLY | os.O_CREATE - if append { + if file.Append { flag |= os.O_APPEND } - f, err := os.OpenFile(fileName, flag, initPermission) + f, err := os.OpenFile(file.Path, flag, initPermission) if err != nil { return err } defer f.Close() - _, err = f.WriteString(fileContent) + _, err = f.WriteString(file.Content) if err != nil { return err } @@ -64,10 +64,10 @@ func (w FileWriter) WriteToFile(fileName string, fileContent string, filePermiss if err != nil { return err } - if len(filePermission) > 0 { + if len(file.Permissions) > 0 { isRootOrFileOwner := false if u.Username != "root" { - stats, err := os.Stat(fileName) + stats, err := os.Stat(file.Path) if err != nil { return err } @@ -82,9 +82,9 @@ func (w FileWriter) WriteToFile(fileName string, fileContent string, filePermiss //Fetch permission information if isRootOrFileOwner { //Make sure agent run as root or file owner - fileMode, err := strconv.ParseUint(filePermission, 8, 32) + fileMode, err := strconv.ParseUint(file.Permissions, 8, 32) if err != nil { - return errors.Wrap(err, fmt.Sprintf("Error parse the file permission %s", filePermission)) + return errors.Wrap(err, fmt.Sprintf("Error parse the file permission %s", file.Permissions)) } err = f.Chmod(fs.FileMode(fileMode)) @@ -93,7 +93,7 @@ func (w FileWriter) WriteToFile(fileName string, fileContent string, filePermiss } } else { //Make sure current user is sudoer, and there is "sudo" and "chmod" command in system - cmd := fmt.Sprintf("sudo chmod %s %s", filePermission, fileName) + cmd := fmt.Sprintf("sudo chmod %s %s", file.Permissions, file.Path) command := exec.Command("/bin/sh", "-c", cmd) _, err := command.Output() if err != nil { @@ -102,14 +102,14 @@ func (w FileWriter) WriteToFile(fileName string, fileContent string, filePermiss } } - if len(fileOwner) > 0 { + if len(file.Owner) > 0 { //Fetch owner information if u.Username == "root" { //only root can do this - owner := strings.Split(fileOwner, ":") + owner := strings.Split(file.Owner, ":") if len(owner) != 2 { - return errors.Wrap(err, fmt.Sprintf("Invalid owner format '%s'", fileOwner)) + return errors.Wrap(err, fmt.Sprintf("Invalid owner format '%s'", file.Owner)) } userInfo, err := user.Lookup(owner[0]) @@ -133,7 +133,7 @@ func (w FileWriter) WriteToFile(fileName string, fileContent string, filePermiss } } else { //Make sure current user is sudoer, and there is "sudo" and "chown" command in system - cmd := fmt.Sprintf("sudo chown %s %s", fileOwner, fileName) + cmd := fmt.Sprintf("sudo chown %s %s", file.Owner, file.Path) command := exec.Command("/bin/sh", "-c", cmd) _, err := command.Output() if err != nil { diff --git a/agent/cloudinit/file_writer_test.go b/agent/cloudinit/file_writer_test.go index b932d5372..b6ebc29cf 100644 --- a/agent/cloudinit/file_writer_test.go +++ b/agent/cloudinit/file_writer_test.go @@ -44,36 +44,47 @@ var _ = Describe("FileWriter", func() { }) It("Should create and write to file", func() { - fileName := path.Join(workDir, "file1.txt") - fileContent := "some-content" + file := Files{ + Path: path.Join(workDir, "file1.txt"), + Encoding: "", + Owner: "", + Permissions: "", + Content: "some-content", + Append: false, + } err := FileWriter{}.MkdirIfNotExists(workDir) Expect(err).ToNot(HaveOccurred()) - err = FileWriter{}.WriteToFile(fileName, fileContent, "", "", false) + err = FileWriter{}.WriteToFile(file) Expect(err).NotTo(HaveOccurred()) - buffer, err := ioutil.ReadFile(fileName) + buffer, err := ioutil.ReadFile(file.Path) Expect(err).NotTo(HaveOccurred()) - Expect(string(buffer)).To(Equal(fileContent)) + Expect(string(buffer)).To(Equal(file.Content)) }) It("Should create and write to file with correct attributes", func() { - - fileName := path.Join(workDir, "file2.txt") userName := "root" groupName := "root" - fileContent := "some-content" filePermission := 0777 + file := Files{ + Path: path.Join(workDir, "file2.txt"), + Encoding: "", + Owner: userName + ":" + groupName, + Permissions: strconv.FormatInt(int64(filePermission), 8), + Content: "some-content", + Append: false, + } err := FileWriter{}.MkdirIfNotExists(workDir) Expect(err).ToNot(HaveOccurred()) - err = FileWriter{}.WriteToFile(fileName, fileContent, strconv.FormatInt(int64(filePermission), 8), userName+":"+groupName, false) + err = FileWriter{}.WriteToFile(file) Expect(err).NotTo(HaveOccurred()) - stats, err := os.Stat(fileName) + stats, err := os.Stat(file.Path) Expect(err).NotTo(HaveOccurred()) Expect(stats.Mode()).To(Equal(fs.FileMode(filePermission))) @@ -90,22 +101,31 @@ var _ = Describe("FileWriter", func() { }) It("Should append content to file when append mode is enabled", func() { - fileName := path.Join(workDir, "file3.txt") + //fileName := path.Join(workDir, "file3.txt") fileOriginContent := "some-content-1" - fileAppendContent := "some-content-2" + //fileAppendContent := "some-content-2" + + file := Files{ + Path: path.Join(workDir, "file3.txt"), + Encoding: "", + Owner: "", + Permissions: "", + Content: "some-content-2", + Append: true, + } err := FileWriter{}.MkdirIfNotExists(workDir) Expect(err).NotTo(HaveOccurred()) - err = ioutil.WriteFile(fileName, []byte(fileOriginContent), 0644) + err = ioutil.WriteFile(file.Path, []byte(fileOriginContent), 0644) Expect(err).NotTo(HaveOccurred()) - err = FileWriter{}.WriteToFile(fileName, fileAppendContent, "", "", true) + err = FileWriter{}.WriteToFile(file) Expect(err).NotTo(HaveOccurred()) - buffer, err := ioutil.ReadFile(fileName) + buffer, err := ioutil.ReadFile(file.Path) Expect(err).NotTo(HaveOccurred()) - Expect(string(buffer)).To(Equal(fileOriginContent + fileAppendContent)) + Expect(string(buffer)).To(Equal(fileOriginContent + file.Content)) }) }) diff --git a/agent/host_agent_test.go b/agent/host_agent_test.go index cc38f5488..21284f760 100644 --- a/agent/host_agent_test.go +++ b/agent/host_agent_test.go @@ -122,9 +122,31 @@ var _ = Describe("Agent", func() { machineName := "test-machine-1" byoMachineName := "test-byomachine-1" - fileName := path.Join(workDir, "file.txt") - fileOriginContent := "some-content-1" - fileNewContent := " run cmd" + fileName1 := path.Join(workDir, "file-1.txt") + fileOriginContent1 := "some-content-1" + fileNewContent1 := " run cmd" + + fileName2 := path.Join(workDir, "file-2.txt") + fileOriginContent2 := "some-content-2" + fileAppendContent2 := "some-content-append-2" + filePermission2 := 0777 + userName2 := "root" + groupName2 := "root" + isAppend2 := true + + fileName3 := path.Join(workDir, "file-3.txt") + fileContent3 := "some-content-3" + fileBase64Content3 := base64.StdEncoding.EncodeToString([]byte(fileContent3)) + + fileName4 := path.Join(workDir, "file-4.txt") + fileContent4 := "some-content-4" + fileGzipContent4, err := gZipData([]byte(fileContent4)) + Expect(err).ToNot(HaveOccurred()) + fileGzipBase64Content4 := base64.StdEncoding.EncodeToString(fileGzipContent4) + + //Init second file + err = ioutil.WriteFile(fileName2, []byte(fileOriginContent2), 0644) + Expect(err).NotTo(HaveOccurred()) byoHost := &infrastructurev1alpha4.ByoHost{} byoHostLookupKey := types.NamespacedName{Name: hostName, Namespace: ns.Name} @@ -136,8 +158,19 @@ var _ = Describe("Agent", func() { bootstrapSecretUnencoded := fmt.Sprintf(`write_files: - path: %s content: %s +- path: %s + owner: %s:%s + permissions: '%s' + content: %s + append: %v +- path: %s + content: %s + encoding: base64 +- path: %s + encoding: gzip+base64 + content: %s runCmd: -- echo -n '%s' >> %s`, fileName, fileOriginContent, fileNewContent, fileName) +- echo -n '%s' >> %s`, fileName1, fileOriginContent1, fileName2, userName2, groupName2, strconv.FormatInt(int64(filePermission2), 8), fileAppendContent2, isAppend2, fileName3, fileBase64Content3, fileName4, fileGzipBase64Content4, fileNewContent1, fileName1) secret, err := createSecret(bootstrapSecretName, bootstrapSecretUnencoded, ns.Name) Expect(err).ToNot(HaveOccurred()) @@ -175,95 +208,42 @@ runCmd: return corev1.ConditionFalse }).Should(Equal(corev1.ConditionTrue)) + //check first file's content Eventually(func() string { - buffer, err := ioutil.ReadFile(fileName) + buffer, err := ioutil.ReadFile(fileName1) if err != nil { return "" } return string(buffer) - }).Should(Equal(fileOriginContent + fileNewContent)) - - }) - - It("check if file created by bootstrap script in correct attributes", func() { - bootstrapSecretName := "bootstrap-secret-2" - machineName := "test-machine-2" - byoMachineName := "test-byomachine-2" - fileName := path.Join(workDir, "file-2.txt") - fileOriginContent := "some-content-2" - fileAppendContent := "some-content-append" - filePermission := 0777 - userName := "root" - groupName := "root" - isAppend := true - - //Init file - err = ioutil.WriteFile(fileName, []byte(fileOriginContent), 0644) - Expect(err).NotTo(HaveOccurred()) - - bootstrapSecretUnencoded := fmt.Sprintf(`write_files: -- path: %s - owner: %s:%s - permissions: '%s' - content: %s - append: %v`, fileName, userName, groupName, strconv.FormatInt(int64(filePermission), 8), fileAppendContent, isAppend) - - byoHost := &infrastructurev1alpha4.ByoHost{} - byoHostLookupKey := types.NamespacedName{Name: hostName, Namespace: ns.Name} - Eventually(func() bool { - err = k8sClient.Get(context.TODO(), byoHostLookupKey, byoHost) - return err == nil - }).ShouldNot(BeFalse()) - - secret, err := createSecret(bootstrapSecretName, bootstrapSecretUnencoded, ns.Name) - Expect(err).ToNot(HaveOccurred()) - - machine, err := createMachine(&secret.Name, machineName, ns.Name) - Expect(err).ToNot(HaveOccurred()) - - byoMachine, err := createByoMachine(byoMachineName, ns.Name, machine) - Expect(err).ToNot(HaveOccurred()) - - helper, err := patch.NewHelper(byoHost, k8sClient) - Expect(err).ToNot(HaveOccurred()) - - byoHost.Status.MachineRef = &corev1.ObjectReference{ - Kind: "ByoMachine", - Namespace: ns.Name, - Name: byoMachine.Name, - UID: byoMachine.UID, - APIVersion: byoHost.APIVersion, - } - err = helper.Patch(context.TODO(), byoHost) - Expect(err).ToNot(HaveOccurred()) + }).Should(Equal(fileOriginContent1 + fileNewContent1)) - //check file content + //check second file's content Eventually(func() string { - buffer, err := ioutil.ReadFile(fileName) + buffer, err := ioutil.ReadFile(fileName2) if err != nil { return "" } return string(buffer) - }).Should(Equal(fileOriginContent + fileAppendContent)) + }).Should(Equal(fileOriginContent2 + fileAppendContent2)) - //check file permission + //check second file permission Eventually(func() bool { - stats, err := os.Stat(fileName) - if err == nil && stats.Mode() == fs.FileMode(filePermission) { + stats, err := os.Stat(fileName2) + if err == nil && stats.Mode() == fs.FileMode(filePermission2) { return true } return false }).Should(BeTrue()) - //check file owner + //check second file's owner Eventually(func() bool { - stats, err := os.Stat(fileName) + stats, err := os.Stat(fileName2) if err != nil { return false } stat := stats.Sys().(*syscall.Stat_t) - userInfo, err := user.Lookup(userName) + userInfo, err := user.Lookup(userName2) if err != nil { return false } @@ -285,77 +265,23 @@ runCmd: }).Should(BeTrue()) - }) - - It("check if file created by bootstrap script in correct encoding way", func() { - bootstrapSecretName := "bootstrap-secret-3" - machineName := "test-machine-3" - byoMachineName := "test-byomachine-3" - - fileName1 := path.Join(workDir, "file-3-1.txt") - fileContent1 := "some-content-3-1" - fileBase64Content1 := base64.StdEncoding.EncodeToString([]byte(fileContent1)) - - fileName2 := path.Join(workDir, "file-3-2.txt") - fileContent2 := "some-content-3-2" - fileGzipContent2, err := gZipData([]byte(fileContent2)) - Expect(err).ToNot(HaveOccurred()) - fileGzipBase64Content2 := base64.StdEncoding.EncodeToString(fileGzipContent2) - - bootstrapSecretUnencoded := fmt.Sprintf(`write_files: -- path: %s - content: %s - encoding: base64 -- path: %s - encoding: gzip+base64 - content: %s`, fileName1, fileBase64Content1, fileName2, fileGzipBase64Content2) - - byoHost := &infrastructurev1alpha4.ByoHost{} - byoHostLookupKey := types.NamespacedName{Name: hostName, Namespace: ns.Name} - Eventually(func() bool { - err = k8sClient.Get(context.TODO(), byoHostLookupKey, byoHost) - return err == nil - }).ShouldNot(BeFalse()) - - secret, err := createSecret(bootstrapSecretName, bootstrapSecretUnencoded, ns.Name) - Expect(err).ToNot(HaveOccurred()) - - machine, err := createMachine(&secret.Name, machineName, ns.Name) - Expect(err).ToNot(HaveOccurred()) - - byoMachine, err := createByoMachine(byoMachineName, ns.Name, machine) - Expect(err).ToNot(HaveOccurred()) - - helper, err := patch.NewHelper(byoHost, k8sClient) - Expect(err).ToNot(HaveOccurred()) - - byoHost.Status.MachineRef = &corev1.ObjectReference{ - Kind: "ByoMachine", - Namespace: ns.Name, - Name: byoMachine.Name, - UID: byoMachine.UID, - APIVersion: byoHost.APIVersion, - } - err = helper.Patch(context.TODO(), byoHost) - Expect(err).ToNot(HaveOccurred()) - - //bas64 + //check if third files's content decoded in base64 way successfully Eventually(func() string { - buffer, err := ioutil.ReadFile(fileName1) + buffer, err := ioutil.ReadFile(fileName3) if err != nil { return "" } return string(buffer) - }).Should(Equal(fileContent1)) + }).Should(Equal(fileContent3)) - //gzip+base64 + //check if fourth files's content decoded in gzip+base64 way successfully Eventually(func() string { - buffer, err := ioutil.ReadFile(fileName2) + buffer, err := ioutil.ReadFile(fileName4) if err != nil { return "" } return string(buffer) - }).Should(Equal(fileContent2)) + }).Should(Equal(fileContent4)) }) }) From ed2cfa7249f339d9fcc0d1dda61816709be92156 Mon Sep 17 00:00:00 2001 From: Hui Chen Date: Wed, 21 Jul 2021 10:36:59 +0800 Subject: [PATCH 03/13] comment from jamie Signed-off-by: Hui Chen --- agent/cloudinit/cloudinit_test.go | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/agent/cloudinit/cloudinit_test.go b/agent/cloudinit/cloudinit_test.go index 0bd277a03..216f36963 100644 --- a/agent/cloudinit/cloudinit_test.go +++ b/agent/cloudinit/cloudinit_test.go @@ -7,6 +7,7 @@ import ( "io/ioutil" "os" "path" + "strings" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -20,16 +21,6 @@ var _ = Describe("Cloudinit", func() { err error ) - BeforeEach(func() { - workDir, err = ioutil.TempDir("", "cloudinit_ut") - Expect(err).ToNot(HaveOccurred()) - }) - - AfterEach(func() { - err := os.RemoveAll(workDir) - Expect(err).ToNot(HaveOccurred()) - }) - Context("Testing write_files and runCmd directives of cloudinit", func() { var ( fakeFileWriter *cloudinitfakes.FakeIFileWriter @@ -51,6 +42,9 @@ var _ = Describe("Cloudinit", func() { content: some-content runCmd: - echo 'some run command'`, workDir) + + workDir, err = ioutil.TempDir("", "cloudinit_ut") + Expect(err).ToNot(HaveOccurred()) }) AfterEach(func() { @@ -99,6 +93,9 @@ runCmd: secondFile := fakeFileWriter.WriteToFileArgsForCall(1) Expect(secondFile.Path).To(Equal(fileName2)) Expect(secondFile.Content).To(Equal(fileContent2)) + Expect(secondFile.Permissions).To(Equal(permissions)) + Expect(secondFile.Owner).To(Equal(strings.Join([]string{user, group}, ":"))) + Expect(secondFile.Append).To(BeTrue()) }) From 114aee1b5e6560774ba7f6ffc2a6cc6747dfb7ba Mon Sep 17 00:00:00 2001 From: chen hui Date: Wed, 21 Jul 2021 03:28:11 +0000 Subject: [PATCH 04/13] comment from jamie Signed-off-by: chen hui --- agent/cloudinit/cloudinit.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/agent/cloudinit/cloudinit.go b/agent/cloudinit/cloudinit.go index 020728e0e..088f0959b 100644 --- a/agent/cloudinit/cloudinit.go +++ b/agent/cloudinit/cloudinit.go @@ -45,8 +45,8 @@ func (se ScriptExecutor) Execute(bootstrapScript string) error { return errors.Wrap(err, fmt.Sprintf("Error creating the directory %s", directoryToCreate)) } - encodings := fixEncoding(file.Encoding) - file.Content, err = fixContent(file.Content, encodings) + encodings := parseEncodingScheme(file.Encoding) + file.Content, err = decodeContent(file.Content, encodings) if err != nil { return errors.Wrap(err, fmt.Sprintf("error decoding content for %s", file.Path)) } @@ -65,7 +65,7 @@ func (se ScriptExecutor) Execute(bootstrapScript string) error { return nil } -func fixEncoding(e string) []string { +func parseEncodingScheme(e string) []string { e = strings.ToLower(e) e = strings.TrimSpace(e) @@ -79,7 +79,7 @@ func fixEncoding(e string) []string { return []string{"text/plain"} } -func fixContent(content string, encodings []string) (string, error) { +func decodeContent(content string, encodings []string) (string, error) { for _, e := range encodings { switch e { case "application/base64": From 0d020d6b0afb1737397addd21c5affa439c9e1aa Mon Sep 17 00:00:00 2001 From: chen hui Date: Thu, 22 Jul 2021 05:10:29 +0000 Subject: [PATCH 05/13] comment from anusha Signed-off-by: chen hui --- agent/host_agent_test.go | 4 ++-- agent/main.go | 19 ++++++++++++------- agent/reconciler/host_reconciler.go | 10 ++++++---- 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/agent/host_agent_test.go b/agent/host_agent_test.go index 21284f760..d9d9101f2 100644 --- a/agent/host_agent_test.go +++ b/agent/host_agent_test.go @@ -60,14 +60,14 @@ var _ = Describe("Agent", func() { command := exec.Command(pathToHostAgentBinary, "--kubeconfig", kubeconfigFile.Name(), "--namespace", ns.Name) session, err = gexec.Start(command, GinkgoWriter, GinkgoWriter) Expect(err).ToNot(HaveOccurred()) - Eventually(session).Should(gexec.Exit(255)) + Eventually(session).Should(gexec.Exit(0)) }) It("should return an error when invalid kubeconfig is passed in", func() { command := exec.Command(pathToHostAgentBinary, "--kubeconfig", fakedKubeConfig) session, err = gexec.Start(command, GinkgoWriter, GinkgoWriter) Expect(err).ToNot(HaveOccurred()) - Eventually(session).Should(gexec.Exit(255)) + Eventually(session).Should(gexec.Exit(0)) }) }) diff --git a/agent/main.go b/agent/main.go index a428cf437..02f62ed38 100644 --- a/agent/main.go +++ b/agent/main.go @@ -4,7 +4,6 @@ import ( "flag" "os" - "github.com/pkg/errors" "github.com/vmware-tanzu/cluster-api-provider-byoh/agent/reconciler" "github.com/vmware-tanzu/cluster-api-provider-byoh/agent/registration" infrastructurev1alpha4 "github.com/vmware-tanzu/cluster-api-provider-byoh/api/v1alpha4" @@ -37,22 +36,26 @@ func main() { config, err := ctrl.GetConfig() if err != nil { - klog.Fatal(err) + klog.Errorf("ctrl.GetConfig return failed, err=%v", err) + return } k8sClient, err := client.New(config, client.Options{Scheme: scheme}) if err != nil { - klog.Fatal(err) + klog.Errorf("client.New return failed, err=%v", err) + return } hostName, err := os.Hostname() if err != nil { - klog.Fatal(errors.Wrap(err, "couldn't determine hostname")) + klog.Errorf("couldn't determine hostname, err=%v", err) + return } err = registration.HostRegistrar{K8sClient: k8sClient}.Register(hostName, namespace) if err != nil { - klog.Fatal(err) + klog.Errorf("Register(%s, %s) return failed, err=%v", hostName, namespace, err) + return } mgr, err := ctrl.NewManager(config, ctrl.Options{ @@ -60,7 +63,8 @@ func main() { Namespace: namespace, }) if err != nil { - klog.Fatal(errors.Wrap(err, "unable to start manager")) + klog.Errorf("unable to start manager, err=%v", err) + return } hostReconciler := reconciler.HostReconciler{Client: k8sClient} @@ -69,6 +73,7 @@ func main() { Complete(hostReconciler) if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil { - klog.Fatal(errors.Wrap(err, "problem running manager")) + klog.Errorf("problem running manager, err=%v", err) + return } } diff --git a/agent/reconciler/host_reconciler.go b/agent/reconciler/host_reconciler.go index 6e1ee900d..77a2fd248 100644 --- a/agent/reconciler/host_reconciler.go +++ b/agent/reconciler/host_reconciler.go @@ -26,7 +26,7 @@ func (r HostReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.R byoHost := &infrastructurev1alpha4.ByoHost{} err := r.Client.Get(ctx, req.NamespacedName, byoHost) if err != nil { - klog.Fatal(err) + klog.Errorf("Get byoHost(%s/%s) failed, err=%v", req.NamespacedName.Namespace, req.NamespacedName.Name, err) return ctrl.Result{}, err } @@ -37,6 +37,7 @@ func (r HostReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.R bootstrapScript, err := r.getBootstrapScript(ctx, byoHost.Status.MachineRef.Name, byoHost.Status.MachineRef.Namespace) if err != nil { + klog.Errorf("getBootstrapScript(%s, %s) return failed, failed, err=%v", byoHost.Status.MachineRef.Name, byoHost.Status.MachineRef.Namespace, err) return ctrl.Result{}, err } @@ -44,18 +45,19 @@ func (r HostReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.R WriteFilesExecutor: cloudinit.FileWriter{}, RunCmdExecutor: cloudinit.CmdRunner{}}.Execute(bootstrapScript) if err != nil { - klog.Fatal(err) + klog.Errorf("cloudinit.ScriptExecutor return failed, err=%v", err) return ctrl.Result{}, err } helper, err := patch.NewHelper(byoHost, r.Client) if err != nil { - klog.Fatal(err) + klog.Errorf("patch.NewHelper return failed, err=%v", err) + return ctrl.Result{}, err } conditions.MarkTrue(byoHost, infrastructurev1alpha4.K8sComponentsInstalledCondition) err = helper.Patch(ctx, byoHost) if err != nil { - klog.Fatal(err) + klog.Errorf("conditions.MarkTrue return failed, err=%v", err) return ctrl.Result{}, err } From 609692d26ef2628bb2012e111692263063ef5c31 Mon Sep 17 00:00:00 2001 From: chen hui Date: Thu, 22 Jul 2021 15:18:36 +0000 Subject: [PATCH 06/13] comment from jamie and anusha Signed-off-by: chen hui --- agent/cloudinit/cloudinit_test.go | 7 +- agent/cloudinit/file_writer.go | 102 ++++++++-------------------- agent/cloudinit/file_writer_test.go | 39 +---------- agent/host_agent_test.go | 37 +--------- 4 files changed, 33 insertions(+), 152 deletions(-) diff --git a/agent/cloudinit/cloudinit_test.go b/agent/cloudinit/cloudinit_test.go index 216f36963..657e98c2e 100644 --- a/agent/cloudinit/cloudinit_test.go +++ b/agent/cloudinit/cloudinit_test.go @@ -7,7 +7,6 @@ import ( "io/ioutil" "os" "path" - "strings" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -61,8 +60,6 @@ runCmd: fileName2 := path.Join(fileDir2, "file2.txt") fileContent2 := "some-content-2" fileBase64Content := base64.StdEncoding.EncodeToString([]byte(fileContent2)) - user := "root" - group := "root" permissions := "0777" encoding := "base64" @@ -71,10 +68,9 @@ runCmd: content: %s - path: %s content: %s - owner: %s:%s permissions: '%s' append: true - encoding: %s`, fileName1, fileContent1, fileName2, fileBase64Content, user, group, permissions, encoding) + encoding: %s`, fileName1, fileContent1, fileName2, fileBase64Content, permissions, encoding) err = scriptExecutor.Execute(bootstrapSecretUnencoded) Expect(err).ToNot(HaveOccurred()) @@ -94,7 +90,6 @@ runCmd: Expect(secondFile.Path).To(Equal(fileName2)) Expect(secondFile.Content).To(Equal(fileContent2)) Expect(secondFile.Permissions).To(Equal(permissions)) - Expect(secondFile.Owner).To(Equal(strings.Join([]string{user, group}, ":"))) Expect(secondFile.Append).To(BeTrue()) }) diff --git a/agent/cloudinit/file_writer.go b/agent/cloudinit/file_writer.go index 8b781221b..4e3e1f596 100644 --- a/agent/cloudinit/file_writer.go +++ b/agent/cloudinit/file_writer.go @@ -4,11 +4,9 @@ import ( "fmt" "io/fs" "os" - "os/exec" "os/user" "strconv" "strings" - "syscall" "github.com/pkg/errors" ) @@ -60,85 +58,43 @@ func (w FileWriter) WriteToFile(file Files) error { return err } - u, err := user.Current() - if err != nil { - return err - } + if len(file.Permissions) > 0 { - isRootOrFileOwner := false - if u.Username != "root" { - stats, err := os.Stat(file.Path) - if err != nil { - return err - } - stat := stats.Sys().(*syscall.Stat_t) - if u.Uid == strconv.FormatUint(uint64(stat.Uid), 10) && u.Gid == strconv.FormatUint(uint64(stat.Gid), 10) { - isRootOrFileOwner = true - } - } else { - isRootOrFileOwner = true + fileMode, err := strconv.ParseUint(file.Permissions, 8, 32) + if err != nil { + return errors.Wrap(err, fmt.Sprintf("Error parse the file permission %s", file.Permissions)) } - //Fetch permission information - if isRootOrFileOwner { - //Make sure agent run as root or file owner - fileMode, err := strconv.ParseUint(file.Permissions, 8, 32) - if err != nil { - return errors.Wrap(err, fmt.Sprintf("Error parse the file permission %s", file.Permissions)) - } - - err = f.Chmod(fs.FileMode(fileMode)) - if err != nil { - return err - } - } else { - //Make sure current user is sudoer, and there is "sudo" and "chmod" command in system - cmd := fmt.Sprintf("sudo chmod %s %s", file.Permissions, file.Path) - command := exec.Command("/bin/sh", "-c", cmd) - _, err := command.Output() - if err != nil { - return err - } + err = f.Chmod(fs.FileMode(fileMode)) + if err != nil { + return err } } if len(file.Owner) > 0 { + owner := strings.Split(file.Owner, ":") + if len(owner) != 2 { + return errors.Wrap(err, fmt.Sprintf("Invalid owner format '%s'", file.Owner)) + } + + userInfo, err := user.Lookup(owner[0]) + if err != nil { + return errors.Wrap(err, fmt.Sprintf("Error Lookup user %s", owner[0])) + } + + uid, err := strconv.ParseUint(userInfo.Uid, 10, 32) + if err != nil { + return errors.Wrap(err, fmt.Sprintf("Error convert uid %s", userInfo.Uid)) + } + + gid, err := strconv.ParseUint(userInfo.Gid, 10, 32) + if err != nil { + return errors.Wrap(err, fmt.Sprintf("Error convert gid %s", userInfo.Gid)) + } - //Fetch owner information - if u.Username == "root" { - //only root can do this - owner := strings.Split(file.Owner, ":") - if len(owner) != 2 { - return errors.Wrap(err, fmt.Sprintf("Invalid owner format '%s'", file.Owner)) - } - - userInfo, err := user.Lookup(owner[0]) - if err != nil { - return errors.Wrap(err, fmt.Sprintf("Error Lookup user %s", owner[0])) - } - - uid, err := strconv.ParseUint(userInfo.Uid, 10, 32) - if err != nil { - return errors.Wrap(err, fmt.Sprintf("Error convert uid %s", userInfo.Uid)) - } - - gid, err := strconv.ParseUint(userInfo.Gid, 10, 32) - if err != nil { - return errors.Wrap(err, fmt.Sprintf("Error convert gid %s", userInfo.Gid)) - } - - err = f.Chown(int(uid), int(gid)) - if err != nil { - return err - } - } else { - //Make sure current user is sudoer, and there is "sudo" and "chown" command in system - cmd := fmt.Sprintf("sudo chown %s %s", file.Owner, file.Path) - command := exec.Command("/bin/sh", "-c", cmd) - _, err := command.Output() - if err != nil { - return err - } + err = f.Chown(int(uid), int(gid)) + if err != nil { + return err } } diff --git a/agent/cloudinit/file_writer_test.go b/agent/cloudinit/file_writer_test.go index b6ebc29cf..8d9cfc26d 100644 --- a/agent/cloudinit/file_writer_test.go +++ b/agent/cloudinit/file_writer_test.go @@ -4,10 +4,8 @@ import ( "io/fs" "io/ioutil" "os" - "os/user" "path" "strconv" - "syscall" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -44,11 +42,12 @@ var _ = Describe("FileWriter", func() { }) It("Should create and write to file", func() { + filePermission := 0777 file := Files{ Path: path.Join(workDir, "file1.txt"), Encoding: "", Owner: "", - Permissions: "", + Permissions: strconv.FormatInt(int64(filePermission), 8), Content: "some-content", Append: false, } @@ -63,48 +62,14 @@ var _ = Describe("FileWriter", func() { Expect(err).NotTo(HaveOccurred()) Expect(string(buffer)).To(Equal(file.Content)) - }) - - It("Should create and write to file with correct attributes", func() { - userName := "root" - groupName := "root" - filePermission := 0777 - file := Files{ - Path: path.Join(workDir, "file2.txt"), - Encoding: "", - Owner: userName + ":" + groupName, - Permissions: strconv.FormatInt(int64(filePermission), 8), - Content: "some-content", - Append: false, - } - - err := FileWriter{}.MkdirIfNotExists(workDir) - Expect(err).ToNot(HaveOccurred()) - - err = FileWriter{}.WriteToFile(file) - Expect(err).NotTo(HaveOccurred()) - stats, err := os.Stat(file.Path) Expect(err).NotTo(HaveOccurred()) Expect(stats.Mode()).To(Equal(fs.FileMode(filePermission))) - userInfo, err := user.Lookup(userName) - Expect(err).NotTo(HaveOccurred()) - uid, err := strconv.ParseUint(userInfo.Uid, 10, 32) - Expect(err).NotTo(HaveOccurred()) - gid, err := strconv.ParseUint(userInfo.Gid, 10, 32) - Expect(err).NotTo(HaveOccurred()) - - stat := stats.Sys().(*syscall.Stat_t) - Expect(stat.Uid).To(Equal(uint32(uid))) - Expect(stat.Gid).To(Equal(uint32(gid))) }) It("Should append content to file when append mode is enabled", func() { - //fileName := path.Join(workDir, "file3.txt") fileOriginContent := "some-content-1" - //fileAppendContent := "some-content-2" - file := Files{ Path: path.Join(workDir, "file3.txt"), Encoding: "", diff --git a/agent/host_agent_test.go b/agent/host_agent_test.go index d9d9101f2..487b4f900 100644 --- a/agent/host_agent_test.go +++ b/agent/host_agent_test.go @@ -10,10 +10,8 @@ import ( "io/ioutil" "os" "os/exec" - "os/user" "path" "strconv" - "syscall" "time" . "github.com/onsi/ginkgo" @@ -130,8 +128,6 @@ var _ = Describe("Agent", func() { fileOriginContent2 := "some-content-2" fileAppendContent2 := "some-content-append-2" filePermission2 := 0777 - userName2 := "root" - groupName2 := "root" isAppend2 := true fileName3 := path.Join(workDir, "file-3.txt") @@ -159,7 +155,6 @@ var _ = Describe("Agent", func() { - path: %s content: %s - path: %s - owner: %s:%s permissions: '%s' content: %s append: %v @@ -170,7 +165,7 @@ var _ = Describe("Agent", func() { encoding: gzip+base64 content: %s runCmd: -- echo -n '%s' >> %s`, fileName1, fileOriginContent1, fileName2, userName2, groupName2, strconv.FormatInt(int64(filePermission2), 8), fileAppendContent2, isAppend2, fileName3, fileBase64Content3, fileName4, fileGzipBase64Content4, fileNewContent1, fileName1) +- echo -n '%s' >> %s`, fileName1, fileOriginContent1, fileName2, strconv.FormatInt(int64(filePermission2), 8), fileAppendContent2, isAppend2, fileName3, fileBase64Content3, fileName4, fileGzipBase64Content4, fileNewContent1, fileName1) secret, err := createSecret(bootstrapSecretName, bootstrapSecretUnencoded, ns.Name) Expect(err).ToNot(HaveOccurred()) @@ -235,36 +230,6 @@ runCmd: return false }).Should(BeTrue()) - //check second file's owner - Eventually(func() bool { - stats, err := os.Stat(fileName2) - if err != nil { - return false - } - stat := stats.Sys().(*syscall.Stat_t) - - userInfo, err := user.Lookup(userName2) - if err != nil { - return false - } - - uid, err := strconv.ParseUint(userInfo.Uid, 10, 32) - if err != nil { - return false - } - - gid, err := strconv.ParseUint(userInfo.Gid, 10, 32) - if err != nil { - return false - } - - if stat.Uid == uint32(uid) && stat.Gid == uint32(gid) { - return true - } - return false - - }).Should(BeTrue()) - //check if third files's content decoded in base64 way successfully Eventually(func() string { buffer, err := ioutil.ReadFile(fileName3) From a8335b5eef31561a368b4473edea64f344817e2f Mon Sep 17 00:00:00 2001 From: huchen2021 <85480625+huchen2021@users.noreply.github.com> Date: Fri, 23 Jul 2021 09:29:27 +0800 Subject: [PATCH 07/13] Update agent/main.go Co-authored-by: Anusha Hegde --- agent/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/main.go b/agent/main.go index 02f62ed38..ce7972dac 100644 --- a/agent/main.go +++ b/agent/main.go @@ -36,7 +36,7 @@ func main() { config, err := ctrl.GetConfig() if err != nil { - klog.Errorf("ctrl.GetConfig return failed, err=%v", err) + klog.Errorf("error getting kubeconfig, err=%v", err) return } From 769149b792b4e8dde81451347387a87febd7465e Mon Sep 17 00:00:00 2001 From: huchen2021 <85480625+huchen2021@users.noreply.github.com> Date: Fri, 23 Jul 2021 09:29:45 +0800 Subject: [PATCH 08/13] Update agent/main.go Co-authored-by: Anusha Hegde --- agent/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/main.go b/agent/main.go index ce7972dac..43252d036 100644 --- a/agent/main.go +++ b/agent/main.go @@ -42,7 +42,7 @@ func main() { k8sClient, err := client.New(config, client.Options{Scheme: scheme}) if err != nil { - klog.Errorf("client.New return failed, err=%v", err) + klog.Errorf("error creating a new k8s client, err=%v", err) return } From 1df9d735f90d2605f0687666b83850b081390c33 Mon Sep 17 00:00:00 2001 From: huchen2021 <85480625+huchen2021@users.noreply.github.com> Date: Fri, 23 Jul 2021 09:30:00 +0800 Subject: [PATCH 09/13] Update agent/reconciler/host_reconciler.go Co-authored-by: Anusha Hegde --- agent/reconciler/host_reconciler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/reconciler/host_reconciler.go b/agent/reconciler/host_reconciler.go index 77a2fd248..2d9c76828 100644 --- a/agent/reconciler/host_reconciler.go +++ b/agent/reconciler/host_reconciler.go @@ -26,7 +26,7 @@ func (r HostReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.R byoHost := &infrastructurev1alpha4.ByoHost{} err := r.Client.Get(ctx, req.NamespacedName, byoHost) if err != nil { - klog.Errorf("Get byoHost(%s/%s) failed, err=%v", req.NamespacedName.Namespace, req.NamespacedName.Name, err) + klog.Errorf("error getting ByoHost %s in namespace %s, err=%v", req.NamespacedName.Namespace, req.NamespacedName.Name, err) return ctrl.Result{}, err } From 414c1e20f89c9a6dd888486d31fac99d5f877d4e Mon Sep 17 00:00:00 2001 From: huchen2021 <85480625+huchen2021@users.noreply.github.com> Date: Fri, 23 Jul 2021 09:30:10 +0800 Subject: [PATCH 10/13] Update agent/reconciler/host_reconciler.go Co-authored-by: Anusha Hegde --- agent/reconciler/host_reconciler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/reconciler/host_reconciler.go b/agent/reconciler/host_reconciler.go index 2d9c76828..59d47b323 100644 --- a/agent/reconciler/host_reconciler.go +++ b/agent/reconciler/host_reconciler.go @@ -51,7 +51,7 @@ func (r HostReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.R helper, err := patch.NewHelper(byoHost, r.Client) if err != nil { - klog.Errorf("patch.NewHelper return failed, err=%v", err) + klog.Errorf("error creating path helper, err=%v", err) return ctrl.Result{}, err } conditions.MarkTrue(byoHost, infrastructurev1alpha4.K8sComponentsInstalledCondition) From 3d5038f136d89f5a6781f6e6b7c6a6a629126bf9 Mon Sep 17 00:00:00 2001 From: huchen2021 <85480625+huchen2021@users.noreply.github.com> Date: Fri, 23 Jul 2021 09:30:19 +0800 Subject: [PATCH 11/13] Update agent/reconciler/host_reconciler.go Co-authored-by: Anusha Hegde --- agent/reconciler/host_reconciler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/reconciler/host_reconciler.go b/agent/reconciler/host_reconciler.go index 59d47b323..44fa89dd8 100644 --- a/agent/reconciler/host_reconciler.go +++ b/agent/reconciler/host_reconciler.go @@ -57,7 +57,7 @@ func (r HostReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.R conditions.MarkTrue(byoHost, infrastructurev1alpha4.K8sComponentsInstalledCondition) err = helper.Patch(ctx, byoHost) if err != nil { - klog.Errorf("conditions.MarkTrue return failed, err=%v", err) + klog.Errorf("error in updating conditions on ByoHost, err=%v", err) return ctrl.Result{}, err } From 054f66ed0d8ca2a63ca9bbb0814d96287d18d324 Mon Sep 17 00:00:00 2001 From: huchen2021 <85480625+huchen2021@users.noreply.github.com> Date: Fri, 23 Jul 2021 09:30:33 +0800 Subject: [PATCH 12/13] Update agent/main.go Co-authored-by: Anusha Hegde --- agent/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/main.go b/agent/main.go index 43252d036..010582b47 100644 --- a/agent/main.go +++ b/agent/main.go @@ -54,7 +54,7 @@ func main() { err = registration.HostRegistrar{K8sClient: k8sClient}.Register(hostName, namespace) if err != nil { - klog.Errorf("Register(%s, %s) return failed, err=%v", hostName, namespace, err) + klog.Errorf("error registering host %s registration in namespace %s, err=%v", hostName, namespace, err) return } From 07ded88161a44b57240b87fa9ddc22e21d9eae72 Mon Sep 17 00:00:00 2001 From: huchen2021 <85480625+huchen2021@users.noreply.github.com> Date: Fri, 23 Jul 2021 09:30:44 +0800 Subject: [PATCH 13/13] Update agent/reconciler/host_reconciler.go Co-authored-by: Anusha Hegde --- agent/reconciler/host_reconciler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/reconciler/host_reconciler.go b/agent/reconciler/host_reconciler.go index 44fa89dd8..d68331a62 100644 --- a/agent/reconciler/host_reconciler.go +++ b/agent/reconciler/host_reconciler.go @@ -37,7 +37,7 @@ func (r HostReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.R bootstrapScript, err := r.getBootstrapScript(ctx, byoHost.Status.MachineRef.Name, byoHost.Status.MachineRef.Namespace) if err != nil { - klog.Errorf("getBootstrapScript(%s, %s) return failed, failed, err=%v", byoHost.Status.MachineRef.Name, byoHost.Status.MachineRef.Namespace, err) + klog.Errorf("error getting bootstrap script for machine %s in namespace %s, err=%v", byoHost.Status.MachineRef.Name, byoHost.Status.MachineRef.Namespace, err) return ctrl.Result{}, err }