Skip to content

Commit

Permalink
MGMT-15243: Skip any zero size manifests when applying (#5355)
Browse files Browse the repository at this point in the history
When applying manifests when installation is started, we should skip any
manifests that are of zero size to prevent them from causing bootstrap
problems later.

We can't validate zero size at the point of file upload as the UI is
presently using an approach where it uploads an empty manifest file to
track that the user wishes to add user defined manifests.

Discussions are underway to change this but for now we need to take this
alternative approach.

This PR implements this.
  • Loading branch information
paul-maidment committed Jul 13, 2023
1 parent 4dfafaf commit 3b0873f
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 2 deletions.
6 changes: 6 additions & 0 deletions internal/ignition/ignition.go
Expand Up @@ -1574,6 +1574,12 @@ func (g *installerGenerator) downloadManifest(ctx context.Context, manifest stri
if err != nil {
return err
}

if len(content) == 0 {
// Ignore any empty files.
return nil
}

// manifest has full path as object-key on s3: clusterID/manifests/[manifests|openshift]/filename
// clusterID/manifests should be trimmed
prefix := manifests.GetManifestObjectName(*g.cluster.ID, "")
Expand Down
19 changes: 17 additions & 2 deletions internal/ignition/ignition_test.go
Expand Up @@ -7,6 +7,7 @@ import (
"errors"
"fmt"
"io"
"io/fs"
"os"
"path/filepath"
"strings"
Expand Down Expand Up @@ -921,15 +922,29 @@ var _ = Describe("downloadManifest", func() {
It("writes the correct file", func() {
ctx := context.Background()
manifestName := fmt.Sprintf("%s/manifests/openshift/masters-chrony-configuration.yaml", cluster.ID)
mockS3Client.EXPECT().Download(ctx, manifestName).Return(io.NopCloser(strings.NewReader("chronyconf")), int64(10), nil)
mockS3Client.EXPECT().Download(ctx, manifestName).Return(io.NopCloser(strings.NewReader("content:entry")), int64(10), nil)
Expect(os.Mkdir(filepath.Join(workDir, "/openshift"), 0755)).To(Succeed())
Expect(os.Mkdir(filepath.Join(workDir, "/manifests"), 0755)).To(Succeed())

Expect(generator.downloadManifest(ctx, manifestName)).To(Succeed())

content, err := os.ReadFile(filepath.Join(workDir, "/openshift/masters-chrony-configuration.yaml"))
Expect(err).NotTo(HaveOccurred())
Expect(content).To(Equal([]byte("chronyconf")))
Expect(content).To(Equal([]byte("content:entry")))
})

It("If a file has empty content, it will not be written", func() {
ctx := context.Background()
manifestName := fmt.Sprintf("%s/manifests/openshift/masters-chrony-configuration.yaml", cluster.ID)
mockS3Client.EXPECT().Download(ctx, manifestName).Return(io.NopCloser(strings.NewReader("")), int64(0), nil)
Expect(os.Mkdir(filepath.Join(workDir, "/openshift"), 0755)).To(Succeed())
Expect(os.Mkdir(filepath.Join(workDir, "/manifests"), 0755)).To(Succeed())

Expect(generator.downloadManifest(ctx, manifestName)).To(Succeed())

_, err := os.Stat(filepath.Join(workDir, "/openshift/masters-chrony-configuration.yaml"))
Expect(err).To(HaveOccurred())
Expect(errors.Is(err, fs.ErrNotExist)).To(BeTrue())
})
})

Expand Down

0 comments on commit 3b0873f

Please sign in to comment.