Skip to content

Commit

Permalink
Remove pgzip and use tar directly
Browse files Browse the repository at this point in the history
This is basically a revert of
#669.

We found that the bosh cli with the pgzip code produces releases that
cannot be compiled by the bosh agent.  In addition, there were many many
edge cases that we found we had to fix and/or paper over that have been
solved in the tar executable over the past decades.

Maybe in the future we can give this another go, as the performance
improvements would be welcome.

Signed-off-by: Ming Xiao <[email protected]>
  • Loading branch information
selzoc authored and mingxiao committed Dec 16, 2024
1 parent b3ce3b8 commit e64bddf
Show file tree
Hide file tree
Showing 43 changed files with 56 additions and 10,200 deletions.
2 changes: 1 addition & 1 deletion cmd/basic_deps.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func NewBasicDepsWithFS(ui *boshui.ConfUI, fs boshsys.FileSystem, logger boshlog

UUIDGen: boshuuid.NewGenerator(),
CmdRunner: cmdRunner,
Compressor: boshcmd.NewTarballCompressor(fs),
Compressor: boshcmd.NewTarballCompressor(cmdRunner, fs),
DigestCalculator: digestCalculator,
DigestCreationAlgorithms: digestCreationAlgorithms,
Time: clock.NewClock(),
Expand Down
5 changes: 2 additions & 3 deletions director/release_archive_with_metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,10 @@ package director

import (
"archive/tar"
"compress/gzip"
"io"
"os"

"github.com/klauspost/pgzip"

"gopkg.in/yaml.v2"

bosherr "github.com/cloudfoundry/bosh-utils/errors"
Expand Down Expand Up @@ -49,7 +48,7 @@ func (a ReleaseArchiveWithMetadata) readMFBytes() ([]byte, error) {

defer file.Close()

gr, err := pgzip.NewReader(file)
gr, err := gzip.NewReader(file)
if err != nil {
return nil, err
}
Expand Down
7 changes: 3 additions & 4 deletions director/release_archive_with_metadata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,9 @@ package director_test
import (
"archive/tar"
"bytes"
"compress/gzip"
"errors"

"github.com/klauspost/pgzip"

fakesys "github.com/cloudfoundry/bosh-utils/system/fakes"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
Expand All @@ -30,7 +29,7 @@ var _ = Describe("NewFSReleaseArchive", func() {

validReleaseTgzBytes := func(fileName, content string) []byte {
fileBytes := &bytes.Buffer{}
gzipWriter := pgzip.NewWriter(fileBytes)
gzipWriter := gzip.NewWriter(fileBytes)
tarWriter := tar.NewWriter(gzipWriter)

{
Expand Down Expand Up @@ -115,7 +114,7 @@ var _ = Describe("NewFSReleaseArchive", func() {

It("returns error if cannot read tar", func() {
fileBytes := &bytes.Buffer{}
gzipWriter := pgzip.NewWriter(fileBytes)
gzipWriter := gzip.NewWriter(fileBytes)

_, err := gzipWriter.Write([]byte("invalid-tar"))
Expect(err).ToNot(HaveOccurred())
Expand Down
5 changes: 2 additions & 3 deletions director/stemcell_archive_with_metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,10 @@ package director

import (
"archive/tar"
"compress/gzip"
"io"
"os"

"github.com/klauspost/pgzip"

"gopkg.in/yaml.v2"

bosherr "github.com/cloudfoundry/bosh-utils/errors"
Expand Down Expand Up @@ -49,7 +48,7 @@ func (a StemcellArchiveWithMetadata) readMFBytes() ([]byte, error) {

defer file.Close()

gr, err := pgzip.NewReader(file)
gr, err := gzip.NewReader(file)
if err != nil {
return nil, err
}
Expand Down
7 changes: 3 additions & 4 deletions director/stemcell_archive_with_metadata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,9 @@ package director_test
import (
"archive/tar"
"bytes"
"compress/gzip"
"errors"

"github.com/klauspost/pgzip"

fakesys "github.com/cloudfoundry/bosh-utils/system/fakes"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
Expand All @@ -30,7 +29,7 @@ var _ = Describe("NewFSStemcellArchive", func() {

validStemcellTgzBytes := func(fileName, content string) []byte {
fileBytes := &bytes.Buffer{}
gzipWriter := pgzip.NewWriter(fileBytes)
gzipWriter := gzip.NewWriter(fileBytes)
tarWriter := tar.NewWriter(gzipWriter)

{
Expand Down Expand Up @@ -115,7 +114,7 @@ var _ = Describe("NewFSStemcellArchive", func() {

It("returns error if cannot read tar", func() {
fileBytes := &bytes.Buffer{}
gzipWriter := pgzip.NewWriter(fileBytes)
gzipWriter := gzip.NewWriter(fileBytes)

_, err := gzipWriter.Write([]byte("invalid-tar"))
Expect(err).ToNot(HaveOccurred())
Expand Down
4 changes: 1 addition & 3 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ require (
github.com/cloudfoundry/bosh-davcli v0.0.385
github.com/cloudfoundry/bosh-gcscli v0.0.266
github.com/cloudfoundry/bosh-s3cli v0.0.336
github.com/cloudfoundry/bosh-utils v0.0.514
github.com/cloudfoundry/bosh-utils v0.0.515
github.com/cloudfoundry/config-server v0.1.224
github.com/cloudfoundry/socks5-proxy v0.2.135
github.com/cppforlife/go-patch v0.2.0
Expand All @@ -21,7 +21,6 @@ require (
github.com/gopacket/gopacket v1.3.1
github.com/hashicorp/go-multierror v1.1.1
github.com/jessevdk/go-flags v1.6.1
github.com/klauspost/pgzip v1.2.6
github.com/mattn/go-isatty v0.0.20
github.com/maxbrunsfeld/counterfeiter/v6 v6.9.0
github.com/onsi/ginkgo/v2 v2.22.0
Expand Down Expand Up @@ -78,7 +77,6 @@ require (
github.com/inconshreveable/mousetrap v1.1.0 // indirect
github.com/jmespath/go-jmespath v0.4.0 // indirect
github.com/jpillora/backoff v1.0.0 // indirect
github.com/klauspost/compress v1.17.11 // indirect
github.com/mattn/go-colorable v0.1.13 // indirect
github.com/mattn/go-runewidth v0.0.16 // indirect
github.com/nu7hatch/gouuid v0.0.0-20131221200532-179d4d0c4d8d // indirect
Expand Down
8 changes: 2 additions & 6 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ github.com/cloudfoundry/bosh-gcscli v0.0.266 h1:/xugLzeTvld6V1FLFUT3vOdqkT4B309D
github.com/cloudfoundry/bosh-gcscli v0.0.266/go.mod h1:eMe1TkdKX2X0bwJFIs8nNxSy79KcxO8drfH8SyR063E=
github.com/cloudfoundry/bosh-s3cli v0.0.336 h1:KdkYBCSEa7NSUL0kIX7I2IDusvKo07YD/zXP8uHk6sk=
github.com/cloudfoundry/bosh-s3cli v0.0.336/go.mod h1:T29octY8qlNT2S+VQHDVNpCEpOwyyEnaBziX+S3P+DE=
github.com/cloudfoundry/bosh-utils v0.0.514 h1:mnBcLVDQsxkEUzaPJ2qMWZfBl5Jxrtj3ewV0PjVKfjg=
github.com/cloudfoundry/bosh-utils v0.0.514/go.mod h1:FRe708iuH+DTFgFhA42ylHKZqQbyAkNuodi+2V/2BRc=
github.com/cloudfoundry/bosh-utils v0.0.515 h1:hAQlg5mGVjHf3f4DKTyIA/wT4l91q7FOb3uIlj+KM3o=
github.com/cloudfoundry/bosh-utils v0.0.515/go.mod h1:Yl7mT/Fy99deXXwaZZVUTf9VC878kUuoTS3h1F9Dzho=
github.com/cloudfoundry/config-server v0.1.224 h1:FJFRDWIa+VJ23u8goO0iNf5GDGI8enmgmFeIvoVuLGU=
github.com/cloudfoundry/config-server v0.1.224/go.mod h1:5+DZnqFi6sDyQ635PRUiym1C3gshl5oOr0Lvaui3bN4=
github.com/cloudfoundry/go-socks5 v0.0.0-20240831012420-2590b55236ee h1:88ruSYvCUKX2YcF2CMYVTmPGITvNdRbzaBRk2c/iMds=
Expand Down Expand Up @@ -150,10 +150,6 @@ github.com/jmespath/go-jmespath/internal/testify v1.5.1 h1:shLQSRRSCCPj3f2gpwzGw
github.com/jmespath/go-jmespath/internal/testify v1.5.1/go.mod h1:L3OGu8Wl2/fWfCI6z80xFu9LTZmf1ZRjMHUOPmWr69U=
github.com/jpillora/backoff v1.0.0 h1:uvFg412JmmHBHw7iwprIxkPMI+sGQ4kzOWsMeHnm2EA=
github.com/jpillora/backoff v1.0.0/go.mod h1:J/6gKK9jxlEcS3zixgDgUAsiuZ7yrSoa/FX5e0EB2j4=
github.com/klauspost/compress v1.17.11 h1:In6xLpyWOi1+C7tXUUWv2ot1QvBjxevKAaI6IXrJmUc=
github.com/klauspost/compress v1.17.11/go.mod h1:pMDklpSncoRMuLFrf1W9Ss9KT+0rH90U12bZKk7uwG0=
github.com/klauspost/pgzip v1.2.6 h1:8RXeL5crjEUFnR2/Sn6GJNWtSQ3Dk8pq4CL3jvdDyjU=
github.com/klauspost/pgzip v1.2.6/go.mod h1:Ch1tH69qFZu15pkjo5kYi6mth2Zzwzt50oCQKQE9RUs=
github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE=
github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk=
github.com/kr/pty v1.1.8 h1:AkaSdXYQOWeaO3neb8EM634ahkXXe3jYbVh/F9lq+GI=
Expand Down
14 changes: 7 additions & 7 deletions integration/create_release_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,13 +308,13 @@ var _ = Describe("release creation", func() {
releaseTarball := listTarballContents(fmt.Sprintf("%s/release.tgz", releaseDir))

expected := []string{
"./release.MF",
"./jobs/",
"./jobs/job1.tgz",
"./packages/",
"./packages/pkg1.tgz",
"./license.tgz",
"./LICENSE",
"release.MF",
"jobs/",
"jobs/job1.tgz",
"packages/",
"packages/pkg1.tgz",
"license.tgz",
"LICENSE",
}
Expect(releaseTarball).To(Equal(expected))
})
Expand Down
8 changes: 4 additions & 4 deletions integration/finalize_release_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ blobstore:

fs.FileExists(releaseDir + expectedLicenseVersion)
releaseTarball := listTarballContents(fmt.Sprintf("%s/release.tgz", releaseDir))
Expect(releaseTarball).To(ContainElement("./LICENSE"))
Expect(releaseTarball).To(ContainElement("LICENSE"))

verifyDigest(releaseDir, expectedLicenseVersion)
})
Expand All @@ -263,7 +263,7 @@ blobstore:

fs.FileExists(releaseDir + expectedLicenseVersion)
releaseTarball := listTarballContents(fmt.Sprintf("%s/release.tgz", releaseDir))
Expect(releaseTarball).To(ContainElement("./NOTICE"))
Expect(releaseTarball).To(ContainElement("NOTICE"))

verifyDigest(releaseDir, expectedLicenseVersion)
})
Expand All @@ -286,8 +286,8 @@ blobstore:

fs.FileExists(releaseDir + expectedLicenseVersion)
releaseTarball := listTarballContents(fmt.Sprintf("%s/release.tgz", releaseDir))
Expect(releaseTarball).To(ContainElement("./NOTICE"))
Expect(releaseTarball).To(ContainElement("./LICENSE"))
Expect(releaseTarball).To(ContainElement("NOTICE"))
Expect(releaseTarball).To(ContainElement("LICENSE"))

verifyDigest(releaseDir, expectedLicenseVersion)
})
Expand Down
2 changes: 1 addition & 1 deletion release/resource/archive_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ var _ = Describe("Archive", func() {
digestCalculator = bicrypto.NewDigestCalculator(fs, []boshcrypto.Algorithm{boshcrypto.DigestAlgorithmSHA1})
fingerprinter := NewFingerprinterImpl(digestCalculator, fs, followSymlinks)
cmdRunner := boshsys.NewExecCmdRunner(logger)
compressor = boshcmd.NewTarballCompressor(fs)
compressor = boshcmd.NewTarballCompressor(cmdRunner, fs)

files := []File{
NewFile(filepath.Join(uniqueDir, "file1"), uniqueDir),
Expand Down
4 changes: 3 additions & 1 deletion templatescompiler/rendered_job_list_compressor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,14 @@ var _ = Describe("RenderedJobListCompressor", func() {
Context("with a real fs & compressor", func() {
var (
fs boshsys.FileSystem
cmdRunner boshsys.CmdRunner
compressor boshcmd.Compressor
)

BeforeEach(func() {
fs = boshsys.NewOsFileSystem(logger)
compressor = boshcmd.NewTarballCompressor(fs)
cmdRunner = boshsys.NewExecCmdRunner(logger)
compressor = boshcmd.NewTarballCompressor(cmdRunner, fs)

renderedJobListCompressor = NewRenderedJobListCompressor(fs, compressor, fakeSHA1Calculator, logger)
})
Expand Down
Loading

0 comments on commit e64bddf

Please sign in to comment.