From 0ef6bf060f1ecc301f7384885941af69a8c8c73a Mon Sep 17 00:00:00 2001 From: Dave Walter Date: Tue, 10 Oct 2017 10:26:15 -0700 Subject: [PATCH] Allows migrations-directory to be specified multiple times [#151810891] Signed-off-by: Rizwan Reza --- .../extra-migrations/v1/some_migration.js | 1 + acceptance/help_test.go | 2 +- acceptance/main_test.go | 11 ++++ builder/tile_writer.go | 10 ++-- builder/tile_writer_test.go | 42 ++++++++++----- commands/bake.go | 4 +- commands/bake_config.go | 2 +- commands/bake_test.go | 54 +++++++++++++++---- kiln/tile_maker_test.go | 4 +- 9 files changed, 97 insertions(+), 33 deletions(-) create mode 100644 acceptance/fixtures/extra-migrations/v1/some_migration.js diff --git a/acceptance/fixtures/extra-migrations/v1/some_migration.js b/acceptance/fixtures/extra-migrations/v1/some_migration.js new file mode 100644 index 000000000..b60af948c --- /dev/null +++ b/acceptance/fixtures/extra-migrations/v1/some_migration.js @@ -0,0 +1 @@ +some_migration diff --git a/acceptance/help_test.go b/acceptance/help_test.go index 01c2fdeea..5511bbb97 100644 --- a/acceptance/help_test.go +++ b/acceptance/help_test.go @@ -28,7 +28,7 @@ Usage: kiln [options] bake [] Command Arguments: -rt, --release-tarball slice location of the release tarball - -m, --migrations-directory string path to the migrations directory + -m, --migrations-directory slice path to the migrations directory -cm, --content-migration slice location of the content migration file -bcm, --base-content-migration string location of the base content migration file -st, --stemcell-tarball string location of the stemcell tarball diff --git a/acceptance/main_test.go b/acceptance/main_test.go index 853c85bad..6ee2d829d 100644 --- a/acceptance/main_test.go +++ b/acceptance/main_test.go @@ -259,6 +259,7 @@ property_blueprints: "--release-tarball", cfReleaseTarball, "--stemcell-tarball", stemcellTarball, "--handcraft", handcraft, + "--migrations-directory", "fixtures/extra-migrations", "--migrations-directory", "fixtures/migrations", "--version", "7.8.9-build.4", "--final-version", "7.8.9", @@ -283,6 +284,7 @@ property_blueprints: var ( archivedMigration1 io.ReadCloser archivedMigration2 io.ReadCloser + archivedMigration3 io.ReadCloser ) for _, f := range zr.File { @@ -295,6 +297,11 @@ property_blueprints: archivedMigration2, err = f.Open() Expect(err).NotTo(HaveOccurred()) } + + if f.Name == "migrations/v1/some_migration.js" { + archivedMigration3, err = f.Open() + Expect(err).NotTo(HaveOccurred()) + } } contents, err := ioutil.ReadAll(archivedMigration1) @@ -304,6 +311,10 @@ property_blueprints: contents, err = ioutil.ReadAll(archivedMigration2) Expect(err).NotTo(HaveOccurred()) Expect(string(contents)).To(Equal("auth-enterprise-sso-migration\n")) + + contents, err = ioutil.ReadAll(archivedMigration3) + Expect(err).NotTo(HaveOccurred()) + Expect(string(contents)).To(Equal("some_migration\n")) }) It("logs the progress to stdout", func() { diff --git a/builder/tile_writer.go b/builder/tile_writer.go index 10f563248..5d820d971 100644 --- a/builder/tile_writer.go +++ b/builder/tile_writer.go @@ -81,10 +81,12 @@ func (w TileWriter) Write(metadataContents []byte, config commands.BakeConfig) e return err } - if config.MigrationsDirectory != "" { - err = w.addMigrations(files, config.MigrationsDirectory) - if err != nil { - return err + if len(config.MigrationDirectories) > 0 { + for _, migrationsDir := range config.MigrationDirectories { + err = w.addMigrations(files, migrationsDir) + if err != nil { + return err + } } } diff --git a/builder/tile_writer_test.go b/builder/tile_writer_test.go index 303792616..ab25e1a4a 100644 --- a/builder/tile_writer_test.go +++ b/builder/tile_writer_test.go @@ -42,7 +42,7 @@ var _ = Describe("TileWriter", func() { ProductName: "cool-product-name", FilenamePrefix: "cool-product-file", ReleaseTarballs: []string{"/some/path/release-1.tgz", "/some/path/release-2.tgz"}, - MigrationsDirectory: "/some/path/migrations", + MigrationDirectories: []string{"/some/path/migrations", "/some/other/path/migrations"}, ContentMigrations: []string{"/some/path/content-migration-1.yml", "/some/path/content-migration-2.yml"}, BaseContentMigration: "/some/path/base-content-migration.yml", Version: "1.2.3-build.4", @@ -59,9 +59,17 @@ var _ = Describe("TileWriter", func() { migrationInfo.IsDirReturns(false) filesystem.WalkStub = func(root string, walkFn filepath.WalkFunc) error { - walkFn("/some/path/migrations", dirInfo, nil) - walkFn("/some/path/migrations/migration-1.js", migrationInfo, nil) - walkFn("/some/path/migrations/migration-2.js", migrationInfo, nil) + switch root { + case "/some/path/migrations": + walkFn("/some/path/migrations", dirInfo, nil) + walkFn("/some/path/migrations/migration-1.js", migrationInfo, nil) + walkFn("/some/path/migrations/migration-2.js", migrationInfo, nil) + case "/some/other/path/migrations": + walkFn("/some/other/path/migrations", dirInfo, nil) + walkFn("/some/other/path/migrations/other-migration.js", migrationInfo, nil) + default: + return nil + } return nil } @@ -75,6 +83,8 @@ var _ = Describe("TileWriter", func() { return NewBuffer(bytes.NewBuffer([]byte("migration-1"))), nil case "/some/path/migrations/migration-2.js": return NewBuffer(bytes.NewBuffer([]byte("migration-2"))), nil + case "/some/other/path/migrations/other-migration.js": + return NewBuffer(bytes.NewBuffer([]byte("other-migration"))), nil default: return nil, fmt.Errorf("open %s: no such file or directory", path) } @@ -93,7 +103,7 @@ var _ = Describe("TileWriter", func() { Expect(zipper.SetPathCall.CallCount).To(Equal(1)) Expect(zipper.SetPathCall.Receives.Path).To(Equal("cool-product-file-1.2.3-build.4.pivotal")) - Expect(zipper.AddCall.Calls).To(HaveLen(6)) + Expect(zipper.AddCall.Calls).To(HaveLen(7)) Expect(zipper.AddCall.Calls[0].Path).To(Equal(filepath.Join("content_migrations", "migrations.yml"))) Eventually(gbytes.BufferReader(zipper.AddCall.Calls[0].File)).Should(gbytes.Say("combined-content-migration-contents")) @@ -107,11 +117,14 @@ var _ = Describe("TileWriter", func() { Expect(zipper.AddCall.Calls[3].Path).To(Equal(filepath.Join("migrations", "v1", "migration-2.js"))) Eventually(gbytes.BufferReader(zipper.AddCall.Calls[3].File)).Should(gbytes.Say("migration-2")) - Expect(zipper.AddCall.Calls[4].Path).To(Equal(filepath.Join("releases", "release-1.tgz"))) - Eventually(gbytes.BufferReader(zipper.AddCall.Calls[4].File)).Should(gbytes.Say(release1Content)) + Expect(zipper.AddCall.Calls[4].Path).To(Equal(filepath.Join("migrations", "v1", "other-migration.js"))) + Eventually(gbytes.BufferReader(zipper.AddCall.Calls[4].File)).Should(gbytes.Say("other-migration")) + + Expect(zipper.AddCall.Calls[5].Path).To(Equal(filepath.Join("releases", "release-1.tgz"))) + Eventually(gbytes.BufferReader(zipper.AddCall.Calls[5].File)).Should(gbytes.Say(release1Content)) - Expect(zipper.AddCall.Calls[5].Path).To(Equal(filepath.Join("releases", "release-2.tgz"))) - Eventually(gbytes.BufferReader(zipper.AddCall.Calls[5].File)).Should(gbytes.Say(release2Content)) + Expect(zipper.AddCall.Calls[6].Path).To(Equal(filepath.Join("releases", "release-2.tgz"))) + Eventually(gbytes.BufferReader(zipper.AddCall.Calls[6].File)).Should(gbytes.Say(release2Content)) Expect(zipper.CloseCall.CallCount).To(Equal(1)) @@ -122,6 +135,7 @@ var _ = Describe("TileWriter", func() { "Adding metadata/cool-product-name.yml to .pivotal...", "Adding migrations/v1/migration-1.js to .pivotal...", "Adding migrations/v1/migration-2.js to .pivotal...", + "Adding migrations/v1/other-migration.js to .pivotal...", "Adding releases/release-1.tgz to .pivotal...", "Adding releases/release-2.tgz to .pivotal...", "Calculated md5 sum: THIS-IS-THE-SUM", @@ -140,7 +154,7 @@ var _ = Describe("TileWriter", func() { ProductName: "cool-product-name", FilenamePrefix: "cool-product-file", ReleaseTarballs: []string{"/some/path/release-1.tgz", "/some/path/release-2.tgz"}, - MigrationsDirectory: "", + MigrationDirectories: []string{}, ContentMigrations: []string{}, BaseContentMigration: "", Version: "1.2.3-build.4", @@ -177,7 +191,7 @@ var _ = Describe("TileWriter", func() { ProductName: "cool-product-name", FilenamePrefix: "cool-product-file", ReleaseTarballs: []string{"/some/path/release-1.tgz", "/some/path/release-2.tgz"}, - MigrationsDirectory: "/some/path/migrations", + MigrationDirectories: []string{"/some/path/migrations"}, ContentMigrations: []string{}, BaseContentMigration: "", Version: "1.2.3-build.4", @@ -250,9 +264,9 @@ var _ = Describe("TileWriter", func() { } config := commands.BakeConfig{ - ReleaseTarballs: []string{"/some/path/release-1.tgz"}, - MigrationsDirectory: "/some/path/migrations", - StubReleases: true, + ReleaseTarballs: []string{"/some/path/release-1.tgz"}, + MigrationDirectories: []string{"/some/path/migrations"}, + StubReleases: true, } err := tileWriter.Write([]byte{}, config) diff --git a/commands/bake.go b/commands/bake.go index 6649e2fe4..e6f180eb9 100644 --- a/commands/bake.go +++ b/commands/bake.go @@ -85,7 +85,7 @@ func (b Bake) parseArgs(args []string) (BakeConfig, error) { return config, errors.New("--output-dir is a required parameter") } - if config.MigrationsDirectory != "" && len(config.ContentMigrations) > 0 { + if len(config.MigrationDirectories) > 0 && len(config.ContentMigrations) > 0 { return config, errors.New("cannot build a tile with content migrations and migrations") } @@ -93,7 +93,7 @@ func (b Bake) parseArgs(args []string) (BakeConfig, error) { return config, errors.New("base content migration is required when content migrations are provided") } - if config.MigrationsDirectory != "" && config.BaseContentMigration != "" { + if len(config.MigrationDirectories) > 0 && config.BaseContentMigration != "" { return config, errors.New("cannot build a tile with a base content migration and migrations") } diff --git a/commands/bake_config.go b/commands/bake_config.go index 769971321..ad717e674 100644 --- a/commands/bake_config.go +++ b/commands/bake_config.go @@ -4,7 +4,7 @@ import "github.com/pivotal-cf/jhanda/flags" type BakeConfig struct { ReleaseTarballs flags.StringSlice `short:"rt" long:"release-tarball" description:"location of the release tarball"` - MigrationsDirectory string `short:"m" long:"migrations-directory" description:"path to the migrations directory"` + MigrationDirectories flags.StringSlice `short:"m" long:"migrations-directory" description:"path to the migrations directory"` ContentMigrations flags.StringSlice `short:"cm" long:"content-migration" description:"location of the content migration file"` BaseContentMigration string `short:"bcm" long:"base-content-migration" description:"location of the base content migration file"` StemcellTarball string `short:"st" long:"stemcell-tarball" description:"location of the stemcell tarball"` diff --git a/commands/bake_test.go b/commands/bake_test.go index 9b6548263..53ee7f119 100644 --- a/commands/bake_test.go +++ b/commands/bake_test.go @@ -70,17 +70,53 @@ var _ = Describe("bake", func() { config := tileMaker.MakeArgsForCall(0) Expect(config).To(Equal(commands.BakeConfig{ - StemcellTarball: "some-stemcell-tarball", - ReleaseTarballs: []string{"some-release-tarball", "some-other-release-tarball"}, - Handcraft: "some-handcraft", - Version: "1.2.3-build.4", - FinalVersion: "1.2.3", - ProductName: "cool-product-name", - FilenamePrefix: "cool-product-file", - OutputDir: "some-output-dir", - MigrationsDirectory: "some-migrations-directory", + StemcellTarball: "some-stemcell-tarball", + ReleaseTarballs: []string{"some-release-tarball", "some-other-release-tarball"}, + Handcraft: "some-handcraft", + Version: "1.2.3-build.4", + FinalVersion: "1.2.3", + ProductName: "cool-product-name", + FilenamePrefix: "cool-product-file", + OutputDir: "some-output-dir", + MigrationDirectories: []string{"some-migrations-directory"}, })) }) + + Context("when the migration directory is specified multiple times", func() { + It("builds the tile", func() { + err := bake.Execute([]string{ + "--stemcell-tarball", "some-stemcell-tarball", + "--release-tarball", "some-release-tarball", + "--release-tarball", "some-other-release-tarball", + "--migrations-directory", "some-migrations-directory", + "--migrations-directory", "some-other-migrations-directory", + "--handcraft", "some-handcraft", + "--version", "1.2.3-build.4", + "--final-version", "1.2.3", + "--product-name", "cool-product-name", + "--filename-prefix", "cool-product-file", + "--output-dir", "some-output-dir", + }) + + Expect(err).NotTo(HaveOccurred()) + Expect(tileMaker.MakeCallCount()).To(Equal(1)) + + config := tileMaker.MakeArgsForCall(0) + Expect(config).To(Equal(commands.BakeConfig{ + StemcellTarball: "some-stemcell-tarball", + ReleaseTarballs: []string{"some-release-tarball", "some-other-release-tarball"}, + Handcraft: "some-handcraft", + Version: "1.2.3-build.4", + FinalVersion: "1.2.3", + ProductName: "cool-product-name", + FilenamePrefix: "cool-product-file", + OutputDir: "some-output-dir", + MigrationDirectories: []string{"some-migrations-directory", "some-other-migrations-directory"}, + })) + + }) + + }) }) It("builds the tile", func() { diff --git a/kiln/tile_maker_test.go b/kiln/tile_maker_test.go index 1fb223cf4..5c8ea6501 100644 --- a/kiln/tile_maker_test.go +++ b/kiln/tile_maker_test.go @@ -35,7 +35,7 @@ var _ = Describe("TileMaker", func() { StemcellTarball: "some-stemcell-tarball", ReleaseTarballs: []string{"some-release-tarball", "some-other-release-tarball"}, Handcraft: "some-handcraft", - MigrationsDirectory: "some-migrations-directory", + MigrationDirectories: []string{"some-migrations-directory"}, BaseContentMigration: "some-base-content-migration", ContentMigrations: []string{"some-content-migration", "some-other-content-migration"}, OutputDir: "some-output-dir", @@ -97,7 +97,7 @@ stemcell_criteria: StemcellTarball: "some-stemcell-tarball", ReleaseTarballs: []string{"some-release-tarball", "some-other-release-tarball"}, Handcraft: "some-handcraft", - MigrationsDirectory: "some-migrations-directory", + MigrationDirectories: []string{"some-migrations-directory"}, BaseContentMigration: "some-base-content-migration", ContentMigrations: []string{"some-content-migration", "some-other-content-migration"}, OutputDir: "some-output-dir",