Skip to content

Commit

Permalink
Remove experimental flag for build drivers
Browse files Browse the repository at this point in the history
Default to buildkit only and remove the docker build driver
Keep the build driver config option, in case we ever get creative in the future

Closes getporter#1954

Signed-off-by: Carolyn Van Slyck <[email protected]>
  • Loading branch information
carolynvs committed Apr 4, 2022
1 parent 922d840 commit 9559431
Show file tree
Hide file tree
Showing 19 changed files with 60 additions and 231 deletions.
9 changes: 3 additions & 6 deletions cmd/porter/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,7 @@ func buildBundleBuildCommand(p *porter.Porter) *cobra.Command {
cmd := &cobra.Command{
Use: "build",
Short: "Build a bundle",
Long: `Builds the bundle in the current directory by generating a Dockerfile and a CNAB bundle.json, and then building the invocation image.
Porter uses the docker driver as the default build driver, an alternate driver may be supplied via --driver or the PORTER_BUILD_DRIVER environment variable.
`,
Long: `Builds the bundle in the current directory by generating a Dockerfile and a CNAB bundle.json, and then building the invocation image.`,
Example: ` porter build
porter build --name newbuns
porter build --version 0.1.0
Expand All @@ -80,8 +77,8 @@ Porter uses the docker driver as the default build driver, an alternate driver m
f.StringVarP(&opts.Dir, "dir", "d", "",
"Path to the build context directory where all bundle assets are located.")
f.StringVar(&opts.Driver, "driver", porter.BuildDriverDefault,
fmt.Sprintf("Experimental. Driver for building the invocation image. Allowed values are: %s", strings.Join(porter.BuildDriverAllowedValues, ", ")))
f.StringSliceVar(&opts.SSH, "ssh", nil,
fmt.Sprintf("Driver for building the invocation image. Allowed values are: %s", strings.Join(porter.BuildDriverAllowedValues, ", ")))
f.MarkHidden("driver") // Hide the driver flag since there aren't any choices to make right now
f.StringArrayVar(&opts.BuildArgs, "build-arg", nil,
"Set build arguments in the template Dockerfile (format: NAME=VALUE). May be specified multiple times.")
f.StringArrayVar(&opts.SSH, "ssh", nil,
Expand Down
5 changes: 1 addition & 4 deletions cmd/porter/bundle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"strings"
"testing"

"get.porter.sh/porter/pkg/experimental"
"get.porter.sh/porter/pkg/porter"
"github.com/spf13/cobra"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -135,8 +134,6 @@ func TestValidateInstallationListCommand(t *testing.T) {

func TestBuildValidate_Driver(t *testing.T) {
// Do not run in parallel
os.Setenv("PORTER_EXPERIMENTAL", experimental.BuildDrivers)
defer os.Unsetenv("PORTER_EXPERIMENTAL")

testcases := []struct {
name string
Expand All @@ -145,7 +142,7 @@ func TestBuildValidate_Driver(t *testing.T) {
wantDriver string
wantError string
}{
{name: "no flag", wantDriver: "docker"},
{name: "no flag", wantDriver: "buildkit"},
{name: "invalid flag", args: "--driver=missing-driver", wantError: "invalid --driver value missing-driver"},
{name: "valid flag", args: "--driver=buildkit", wantDriver: "buildkit"},
{name: "invalid config", args: "", configDriver: "invalid-driver", wantError: "invalid --driver value invalid-driver"},
Expand Down
10 changes: 5 additions & 5 deletions cmd/porter/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,22 +92,22 @@ func TestExperimentalFlags(t *testing.T) {
cmd := buildRootCommandFrom(p.Porter)
cmd.SetArgs([]string{"install"})
cmd.Execute()
assert.False(t, p.Config.IsFeatureEnabled(experimental.FlagBuildDrivers))
assert.False(t, p.Config.IsFeatureEnabled(experimental.FlagStructuredLogs))
})

t.Run("flag set", func(t *testing.T) {
p := porter.NewTestPorter(t)
defer p.Teardown()

cmd := buildRootCommandFrom(p.Porter)
cmd.SetArgs([]string{"install", "--experimental", experimental.BuildDrivers})
cmd.SetArgs([]string{"install", "--experimental", experimental.StructuredLogs})
cmd.Execute()

assert.True(t, p.Config.IsFeatureEnabled(experimental.FlagBuildDrivers))
assert.True(t, p.Config.IsFeatureEnabled(experimental.FlagStructuredLogs))
})

t.Run("flag unset, env set", func(t *testing.T) {
os.Setenv(expEnvVar, experimental.BuildDrivers)
os.Setenv(expEnvVar, experimental.StructuredLogs)
defer os.Unsetenv(expEnvVar)

p := porter.NewTestPorter(t)
Expand All @@ -117,6 +117,6 @@ func TestExperimentalFlags(t *testing.T) {
cmd.SetArgs([]string{"install"})
cmd.Execute()

assert.True(t, p.Config.IsFeatureEnabled(experimental.FlagBuildDrivers))
assert.True(t, p.Config.IsFeatureEnabled(experimental.FlagStructuredLogs))
})
}
4 changes: 0 additions & 4 deletions docs/content/cli/build.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,6 @@ Build a bundle

Builds the bundle in the current directory by generating a Dockerfile and a CNAB bundle.json, and then building the invocation image.

Porter uses the docker driver as the default build driver, an alternate driver may be supplied via --driver or the PORTER_BUILD_DRIVER environment variable.


```
porter build [flags]
```
Expand All @@ -32,7 +29,6 @@ porter build [flags]
### Options

```
--driver string Experimental. Driver for building the invocation image. Allowed values are: buildkit (default "buildkit")
--build-arg stringArray Set build arguments in the template Dockerfile (format: NAME=VALUE). May be specified multiple times.
-d, --dir string Path to the build context directory where all bundle assets are located.
-f, --file porter.yaml Path to the Porter manifest. Defaults to porter.yaml in the current directory.
Expand Down
4 changes: 0 additions & 4 deletions docs/content/cli/bundles_build.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,6 @@ Build a bundle

Builds the bundle in the current directory by generating a Dockerfile and a CNAB bundle.json, and then building the invocation image.

Porter uses the docker driver as the default build driver, an alternate driver may be supplied via --driver or the PORTER_BUILD_DRIVER environment variable.


```
porter bundles build [flags]
```
Expand All @@ -32,7 +29,6 @@ porter bundles build [flags]
### Options

```
--driver string Experimental. Driver for building the invocation image. Allowed values are: buildkit (default "buildkit")
--build-arg stringArray Set build arguments in the template Dockerfile (format: NAME=VALUE). May be specified multiple times.
-d, --dir string Path to the build context directory where all bundle assets are located.
-f, --file porter.yaml Path to the Porter manifest. Defaults to porter.yaml in the current directory.
Expand Down
45 changes: 2 additions & 43 deletions docs/content/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -185,49 +185,8 @@ feature by:

### Build Drivers

The **build-drivers** experimental feature flag enables using a different
driver to build OCI images used by the bundle, such as the installer.

You can set your desired driver with either using `porter build --driver`,
PORTER_BUILD_DRIVER environment variable, or in the configuration file with
build-driver = "DRIVER".

The default driver is docker, and the full list of available drivers
is below:

* **Docker**: Build an OCI image using the [Docker library], without buildkit support.
This requires access to a Docker daemon, either locally or remote.
* **Buildkit**: Build an OCI image using [Docker with Buildkit].
With buildkit you can improve the performance of builds using caching, access
private resources during build, and more.
This requires access to a Docker daemon, either locally or remote.

Below are some examples of how to enable the build-drivers feature and specify an alternate
driver:

**Flags**
```
porter build --experimental build-drivers --driver buildkit
```

**Environment Variables**
```
export PORTER_EXPERIMENTAL=build-drivers
export PORTER_BUILD_DRIVER=buildkit
```

**Configuration File**
```toml
experimental = ["build-drivers"]
build-driver = "buildkit"
```

[install]: /cli/porter_install/
[upgrade]: /cli/porter_upgrade/
[invoke]: /cli/porter_invoke/
[uninstall]: /cli/porter_uninstall/
[Docker library]: https://github.com/moby/moby
[Docker with Buildkit]: https://docs.docker.com/develop/develop-images/build_enhancements/
The **build-drivers** experimental feature flag is no longer used.
Build drivers are enabled by default and the only available driver is buildkit.

### Structured Logs

Expand Down
51 changes: 21 additions & 30 deletions pkg/build/dockerfile-generator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,11 @@ package build

import (
"context"
"fmt"
"path/filepath"
"strings"
"testing"

"get.porter.sh/porter/pkg/config"
"get.porter.sh/porter/pkg/experimental"
"get.porter.sh/porter/pkg/manifest"
"get.porter.sh/porter/pkg/mixin"
"get.porter.sh/porter/pkg/templates"
Expand All @@ -20,34 +18,27 @@ import (
func TestPorter_buildDockerfile(t *testing.T) {
t.Parallel()

drivers := []string{config.BuildDriverDocker, config.BuildDriverBuildkit}
for _, driver := range drivers {
t.Run(driver, func(t *testing.T) {

c := config.NewTestConfig(t)
c.Data.BuildDriver = driver
c.SetExperimentalFlags(experimental.FlagBuildDrivers)
tmpl := templates.NewTemplates(c.Config)
configTpl, err := tmpl.GetManifest()
require.Nil(t, err)
c.TestContext.AddTestFileContents(configTpl, config.Name)

m, err := manifest.LoadManifestFrom(context.Background(), c.Config, config.Name)
require.NoError(t, err, "could not load manifest")

// ignore mixins in the unit tests
m.Mixins = []manifest.MixinDeclaration{}

mp := mixin.NewTestMixinProvider()
g := NewDockerfileGenerator(c.Config, m, tmpl, mp)
gotlines, err := g.buildDockerfile()
require.NoError(t, err)
gotDockerfile := strings.Join(gotlines, "\n")

wantDockerfilePath := fmt.Sprintf("testdata/%s.Dockerfile", driver)
test.CompareGoldenFile(t, wantDockerfilePath, gotDockerfile)
})
}
c := config.NewTestConfig(t)
c.Data.BuildDriver = config.BuildDriverBuildkit
tmpl := templates.NewTemplates(c.Config)
configTpl, err := tmpl.GetManifest()
require.Nil(t, err)
c.TestContext.AddTestFileContents(configTpl, config.Name)

m, err := manifest.LoadManifestFrom(context.Background(), c.Config, config.Name)
require.NoError(t, err, "could not load manifest")

// ignore mixins in the unit tests
m.Mixins = []manifest.MixinDeclaration{}

mp := mixin.NewTestMixinProvider()
g := NewDockerfileGenerator(c.Config, m, tmpl, mp)
gotlines, err := g.buildDockerfile()
require.NoError(t, err)
gotDockerfile := strings.Join(gotlines, "\n")

wantDockerfilePath := "testdata/buildkit.Dockerfile"
test.CompareGoldenFile(t, wantDockerfilePath, gotDockerfile)
}

func TestPorter_buildCustomDockerfile(t *testing.T) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ COPY mybin /cnab/app/

RUN rm ${BUNDLE_DIR}/porter.yaml
RUN rm -fr ${BUNDLE_DIR}/.cnab
COPY .cnab /cnab
COPY --link .cnab /cnab
RUN chgrp -R ${BUNDLE_GID} /cnab && chmod -R g=u /cnab
USER ${BUNDLE_UID}
WORKDIR ${BUNDLE_DIR}
Expand Down
19 changes: 0 additions & 19 deletions pkg/build/testdata/docker.Dockerfile

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ RUN useradd ${BUNDLE_USER} -m -u ${BUNDLE_UID} -g ${BUNDLE_GID} -o

RUN rm ${BUNDLE_DIR}/porter.yaml
RUN rm -fr ${BUNDLE_DIR}/.cnab
COPY .cnab /cnab
COPY --link .cnab /cnab
RUN chgrp -R ${BUNDLE_GID} /cnab && chmod -R g=u /cnab
USER ${BUNDLE_UID}
WORKDIR ${BUNDLE_DIR}
Expand Down
2 changes: 1 addition & 1 deletion pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ func (c *Config) IsFeatureEnabled(flag experimental.FeatureFlags) bool {
}

// SetExperimentalFlags programmatically, overriding Config.Data.ExperimentalFlags.
// Example: Config.SetExperimentalFlags(experimental.FlagBuildDrivers | ...)
// Example: Config.SetExperimentalFlags(experimental.FlagStructuredLogs | ...)
func (c *Config) SetExperimentalFlags(flags experimental.FeatureFlags) {
c.experimental = &flags
}
Expand Down
25 changes: 11 additions & 14 deletions pkg/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,15 @@ func TestConfig_GetHomeDirFromSymlink(t *testing.T) {
func TestConfig_GetFeatureFlags(t *testing.T) {
t.Parallel()

t.Run("build drivers defaulted to disabled", func(t *testing.T) {
t.Run("structured logs defaulted to disabled", func(t *testing.T) {
c := Config{}
assert.False(t, c.IsFeatureEnabled(experimental.FlagBuildDrivers))
assert.False(t, c.IsFeatureEnabled(experimental.FlagStructuredLogs))
})

t.Run("build drivers enabled", func(t *testing.T) {
t.Run("structured logs enabled", func(t *testing.T) {
c := Config{}
c.Data.ExperimentalFlags = []string{experimental.BuildDrivers}
assert.True(t, c.IsFeatureEnabled(experimental.FlagBuildDrivers))
c.Data.ExperimentalFlags = []string{experimental.StructuredLogs}
assert.True(t, c.IsFeatureEnabled(experimental.FlagStructuredLogs))
})
}

Expand All @@ -60,26 +60,23 @@ func TestConfigExperimentalFlags(t *testing.T) {

t.Run("default off", func(t *testing.T) {
c := NewTestConfig(t)
assert.False(t, c.IsFeatureEnabled(experimental.FlagBuildDrivers))
assert.False(t, c.IsFeatureEnabled(experimental.FlagStructuredLogs))
})

t.Run("user configuration", func(t *testing.T) {
os.Setenv("PORTER_EXPERIMENTAL", "build-drivers")
os.Setenv("PORTER_EXPERIMENTAL", experimental.StructuredLogs)
defer os.Unsetenv("PORTER_EXPERIMENTAL")

os.Setenv("PORTER_BUILD_DRIVER", "buildkit")
defer os.Unsetenv("PORTER_BUILD_DRIVER")

c := New()
require.NoError(t, c.Load(context.Background(), nil), "Load failed")
assert.True(t, c.IsFeatureEnabled(experimental.FlagBuildDrivers))
assert.True(t, c.IsFeatureEnabled(experimental.FlagStructuredLogs))
})

t.Run("programmatically enabled", func(t *testing.T) {
c := NewTestConfig(t)
c.Data.ExperimentalFlags = nil
c.SetExperimentalFlags(experimental.FlagBuildDrivers)
assert.True(t, c.IsFeatureEnabled(experimental.FlagBuildDrivers))
c.SetExperimentalFlags(experimental.FlagStructuredLogs)
assert.True(t, c.IsFeatureEnabled(experimental.FlagStructuredLogs))
})
}

Expand All @@ -88,6 +85,6 @@ func TestConfig_GetBuildDriver(t *testing.T) {
c.Data.BuildDriver = "special"
require.Equal(t, BuildDriverBuildkit, c.GetBuildDriver(), "Default to docker when experimental is false, even when a build driver is set")

c.SetExperimentalFlags(experimental.FlagBuildDrivers)
c.SetExperimentalFlags(experimental.FlagStructuredLogs)
require.Equal(t, BuildDriverBuildkit, c.GetBuildDriver(), "Use the specified driver when the build driver feature is enabled")
}
9 changes: 2 additions & 7 deletions pkg/experimental/experimental.go
Original file line number Diff line number Diff line change
@@ -1,27 +1,22 @@
package experimental

const (
// BuildDrivers experimental flag
BuildDrivers = "build-drivers"
StructuredLogs = "structured-logs"
)

// FeatureFlags is an enum of possible feature flags
type FeatureFlags int

const (
// FlagBuildDrivers indicates if configurable build drivers are enabled.
FlagBuildDrivers FeatureFlags = iota + 1
FlagStructuredLogs
// FlagStructuredLogs indicates if structured logs are enabled
FlagStructuredLogs FeatureFlags = iota + 1
)

// ParseFlags converts a list of feature flag names into a bit map for faster lookups.
func ParseFlags(flags []string) FeatureFlags {
var experimental FeatureFlags
for _, flag := range flags {
switch flag {
case BuildDrivers:
experimental = experimental | FlagBuildDrivers
case StructuredLogs:
experimental = experimental | FlagStructuredLogs
}
Expand Down
Loading

0 comments on commit 9559431

Please sign in to comment.