Skip to content

Commit

Permalink
fix: switched from env var to requirements flag
Browse files Browse the repository at this point in the history
lets switch from using an environment variable to enable the experimental helmfile mode to using an optional boolean on the requirements file as its hard to enforce environment variables being consistently set across pipelines/shells/CLI invocations/steps.

The new flag is only marshalled to YAML if its set to true so has no backwards compatibility issues. Its also clearly marked as experimental in the generated reference docs

See: #6442

Signed-off-by: James Strachan <[email protected]>
  • Loading branch information
jstrachan authored and jenkins-x-bot committed Jan 28, 2020
1 parent 3ab4582 commit 2ea94d9
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 15 deletions.
5 changes: 2 additions & 3 deletions pkg/cmd/step/verify/step_verify_preinstall.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"strings"

"github.com/jenkins-x/jx/pkg/cloud/amazon/session"
"github.com/jenkins-x/jx/pkg/features"
"github.com/jenkins-x/jx/pkg/prow"
"sigs.k8s.io/yaml"

Expand Down Expand Up @@ -186,7 +185,7 @@ func (o *StepVerifyPreInstallOptions) Run() error {
return err
}
log.Logger().Info("\n")
if !o.DisableVerifyHelm && !features.IsHelmfile() {
if !o.DisableVerifyHelm && !requirements.Helmfile {
err = o.verifyHelm(ns)
if err != nil {
return err
Expand Down Expand Up @@ -1003,7 +1002,7 @@ func (o *StepVerifyPreInstallOptions) SaveConfig(c *config.RequirementsConfig, f
return errors.Wrapf(err, "failed to save file %s", fileName)
}

if features.IsHelmfile() {
if c.Helmfile {
y := config.RequirementsValues{
RequirementsConfig: c,
}
Expand Down
11 changes: 6 additions & 5 deletions pkg/config/install_requirements.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,11 @@ import (

"github.com/vrischmann/envconfig"

"github.com/imdario/mergo"
"github.com/jenkins-x/jx/pkg/cloud/gke"
"github.com/jenkins-x/jx/pkg/features"

"github.com/ghodss/yaml"
"github.com/imdario/mergo"
v1 "github.com/jenkins-x/jx/pkg/apis/jenkins.io/v1"
"github.com/jenkins-x/jx/pkg/cloud"
"github.com/jenkins-x/jx/pkg/cloud/gke"
"github.com/jenkins-x/jx/pkg/log"
"github.com/pkg/errors"

Expand Down Expand Up @@ -477,6 +475,9 @@ type RequirementsConfig struct {
// GitOps if enabled we will setup a webhook in the boot configuration git repository so that we can
// re-run 'jx boot' when changes merge to the master branch
GitOps bool `json:"gitops,omitempty"`
// Indicates if we are using helmfile and helm 3 to spin up environments. This is currently an experimental
// feature flag used to implement better Multi-Cluster support. See https://github.com/jenkins-x/jx/issues/6442
Helmfile bool `json:"helmfile,omitempty"`
// Kaniko whether to enable kaniko for building docker images
Kaniko bool `json:"kaniko,omitempty"`
// Ingress contains ingress specific requirements
Expand Down Expand Up @@ -648,7 +649,7 @@ func (c *RequirementsConfig) SaveConfig(fileName string) error {
return errors.Wrapf(err, "failed to save file %s", fileName)
}

if features.IsHelmfile() {
if c.Helmfile {
y := RequirementsValues{
RequirementsConfig: c,
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/config/install_requirements_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ func TestRequirementsConfigMarshalExistingFileKanikoFalse(t *testing.T) {

err = requirements.SaveConfig(file)
assert.NoError(t, err, "failed to save file %s", file)
t.Logf("saved file %s", file)

requirements, fileName, err := config.LoadRequirementsConfig(dir)
assert.NoError(t, err, "failed to load requirements file in dir %s", dir)
Expand Down Expand Up @@ -353,7 +354,7 @@ func TestHelmfileRequirementValues(t *testing.T) {
valuesFileExists, err := util.FileExists(valuesFile)
assert.False(t, valuesFileExists, "file %s should not exist", valuesFile)

os.Setenv("JX_HELMFILE", "true")
requirements.Helmfile = true
err = requirements.SaveConfig(file)
assert.NoError(t, err, "failed to save file %s", file)
assert.FileExists(t, file)
Expand Down
6 changes: 0 additions & 6 deletions pkg/features/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package features

import (
"errors"
"os"
"strings"

"reflect"
Expand Down Expand Up @@ -150,8 +149,3 @@ func IsEnabled(cmd *cobra.Command) error {
}
return nil
}

// IsHelmfile returns true if the helmfile feature flag env var is set
func IsHelmfile() bool {
return os.Getenv("JX_HELMFILE") == "true"
}

0 comments on commit 2ea94d9

Please sign in to comment.