Prepare single source of thruth for defined feature flags

parent a76df563
......@@ -15,6 +15,7 @@ import (
"github.com/sirupsen/logrus"
"gitlab.com/gitlab-org/gitlab-runner/helpers"
"gitlab.com/gitlab-org/gitlab-runner/helpers/featureflags"
"gitlab.com/gitlab-org/gitlab-runner/helpers/tls"
"gitlab.com/gitlab-org/gitlab-runner/session"
"gitlab.com/gitlab-org/gitlab-runner/session/terminal"
......@@ -68,14 +69,6 @@ const (
BuildStageUploadOnFailureArtifacts BuildStage = "upload_artifacts_on_failure"
)
const (
FFK8sEntrypointOverCommand string = "FF_K8S_USE_ENTRYPOINT_OVER_COMMAND"
FFDockerHelperImageV2 string = "FF_DOCKER_HELPER_IMAGE_V2"
FFCmdDisableDelayedErrorLevelExpansion string = "FF_CMD_DISABLE_DELAYED_ERROR_LEVEL_EXPANSION"
FFUseLegacyGitCleanStrategy string = "FF_USE_LEGACY_GIT_CLEAN_STRATEGY"
FFUseLegacyBuildsDirForDocker string = "FF_USE_LEGACY_BUILDS_DIR_FOR_DOCKER"
)
type Build struct {
JobResponse `yaml:",inline"`
......@@ -575,13 +568,18 @@ func (b *Build) GetDefaultVariables() JobVariables {
}
func (b *Build) GetDefaultFeatureFlagsVariables() JobVariables {
return JobVariables{
{Key: FFK8sEntrypointOverCommand, Value: "true", Public: true, Internal: true, File: false}, // TODO: Remove in 12.0
{Key: FFDockerHelperImageV2, Value: "false", Public: true, Internal: true, File: false}, // TODO: Remove in 12.0
{Key: FFUseLegacyGitCleanStrategy, Value: "false", Public: true, Internal: true, File: false}, // TODO: Remove in 12.0
{Key: FFCmdDisableDelayedErrorLevelExpansion, Value: "false", Public: true, Internal: true, File: false},
{Key: FFUseLegacyBuildsDirForDocker, Value: "false", Public: true, Internal: true, File: false}, // TODO: Remove in 13.0
variables := make(JobVariables, 0)
for _, featureFlag := range featureflags.GetAll() {
variables = append(variables, JobVariable{
Key: featureFlag.Name,
Value: featureFlag.DefaultValue,
Public: true,
Internal: true,
File: false,
})
}
return variables
}
func (b *Build) GetSharedEnvVariable() JobVariable {
......@@ -838,17 +836,13 @@ func NewBuild(jobData JobResponse, runnerConfig *RunnerConfig, systemInterrupt c
}
func (b *Build) IsFeatureFlagOn(name string) bool {
ffValue := b.GetAllVariables().Get(name)
if ffValue == "" {
return false
}
value := b.GetAllVariables().Get(name)
on, err := strconv.ParseBool(ffValue)
on, err := featureflags.IsOn(value)
if err != nil {
logrus.WithError(err).
WithField("ffName", name).
WithField("ffValue", ffValue).
WithField("name", name).
WithField("value", value).
Error("Error while parsing the value of feature flag")
return false
......
......@@ -29,6 +29,7 @@ import (
"gitlab.com/gitlab-org/gitlab-runner/helpers"
docker_helpers "gitlab.com/gitlab-org/gitlab-runner/helpers/docker"
"gitlab.com/gitlab-org/gitlab-runner/helpers/docker/helperimage"
"gitlab.com/gitlab-org/gitlab-runner/helpers/featureflags"
)
const (
......@@ -270,7 +271,7 @@ func (e *executor) getPrebuiltImage() (*types.ImageInspect, error) {
imageNameFromConfig = common.AppVersion.Variables().ExpandValue(imageNameFromConfig)
e.Debugln("Pull configured helper_image for predefined container instead of import bundled image", imageNameFromConfig, "...")
if !e.Build.IsFeatureFlagOn(common.FFDockerHelperImageV2) {
if !e.Build.IsFeatureFlagOn(featureflags.FFDockerHelperImageV2) {
e.Warningln("DEPRECATION: With gitlab-runner 12.0 we will change some tools inside the helper image, please make sure your image is compliant with the new API. https://gitlab.com/gitlab-org/gitlab-runner/issues/4013")
}
......@@ -389,7 +390,7 @@ func (e *executor) createCacheVolume(containerName, containerPath string) (strin
cmd := []string{"gitlab-runner-helper", "cache-init", containerPath}
// TODO: Remove in 12.0 to start using the command from `gitlab-runner-helper`
if e.checkOutdatedHelperImage() {
e.Debugln(common.FFDockerHelperImageV2, "is not set, falling back to old command")
e.Debugln(featureflags.FFDockerHelperImageV2, "is not set, falling back to old command")
cmd = []string{"gitlab-runner-cache", containerPath}
}
......@@ -508,7 +509,7 @@ func fakeContainer(id string, names ...string) *types.Container {
func (e *executor) createBuildVolume() error {
parentDir := e.Build.RootDir
if e.Build.IsFeatureFlagOn(common.FFUseLegacyBuildsDirForDocker) {
if e.Build.IsFeatureFlagOn(featureflags.FFUseLegacyBuildsDirForDocker) {
// Cache Git sources:
// take path of the projects directory,
// because we use `rm -rf` which could remove the mounted volume
......@@ -1311,7 +1312,7 @@ func (e *executor) runServiceHealthCheckContainer(service *types.Container, time
cmd := []string{"gitlab-runner-helper", "health-check"}
// TODO: Remove in 12.0 to start using the command from `gitlab-runner-helper`
if e.checkOutdatedHelperImage() {
e.Debugln(common.FFDockerHelperImageV2, "is not set, falling back to old command")
e.Debugln(featureflags.FFDockerHelperImageV2, "is not set, falling back to old command")
cmd = []string{"gitlab-runner-service"}
}
......@@ -1417,5 +1418,5 @@ func (e *executor) readContainerLogs(containerID string) string {
}
func (e *executor) checkOutdatedHelperImage() bool {
return !e.Build.IsFeatureFlagOn(common.FFDockerHelperImageV2) && e.Config.Docker.HelperImage != ""
return !e.Build.IsFeatureFlagOn(featureflags.FFDockerHelperImageV2) && e.Config.Docker.HelperImage != ""
}
......@@ -26,6 +26,7 @@ import (
"gitlab.com/gitlab-org/gitlab-runner/helpers"
"gitlab.com/gitlab-org/gitlab-runner/helpers/docker"
"gitlab.com/gitlab-org/gitlab-runner/helpers/docker/helperimage"
"gitlab.com/gitlab-org/gitlab-runner/helpers/featureflags"
)
func TestMain(m *testing.M) {
......@@ -1182,7 +1183,7 @@ func TestCreateCacheVolumeFeatureFlag(t *testing.T) {
{
name: "Helper image is not specified and FF still turned on",
variables: common.JobVariables{
common.JobVariable{Key: common.FFDockerHelperImageV2, Value: "true"},
common.JobVariable{Key: featureflags.FFDockerHelperImageV2, Value: "true"},
},
helperImage: "",
expectedCmd: []string{"gitlab-runner-helper", "cache-init", cacheDir},
......@@ -1196,7 +1197,7 @@ func TestCreateCacheVolumeFeatureFlag(t *testing.T) {
{
name: "Helper image is specified & FF variable is set to true",
variables: common.JobVariables{
common.JobVariable{Key: common.FFDockerHelperImageV2, Value: "true"},
common.JobVariable{Key: featureflags.FFDockerHelperImageV2, Value: "true"},
},
helperImage: "gitlab/gitlab-runner-helper:x86_64-latest",
expectedCmd: []string{"gitlab-runner-helper", "cache-init", cacheDir},
......@@ -1263,7 +1264,7 @@ func TestRunServiceHealthCheckContainerFeatureFlag(t *testing.T) {
{
name: "Helper image is not specified and FF still turned on",
variables: common.JobVariables{
common.JobVariable{Key: common.FFDockerHelperImageV2, Value: "true"},
common.JobVariable{Key: featureflags.FFDockerHelperImageV2, Value: "true"},
},
helperImage: "",
expectedCmd: []string{"gitlab-runner-helper", "health-check"},
......@@ -1277,7 +1278,7 @@ func TestRunServiceHealthCheckContainerFeatureFlag(t *testing.T) {
{
name: "Helper image is specified & FF variable is set to true",
variables: common.JobVariables{
common.JobVariable{Key: common.FFDockerHelperImageV2, Value: "true"},
common.JobVariable{Key: featureflags.FFDockerHelperImageV2, Value: "true"},
},
helperImage: "gitlab/gitlab-runner-helper:x86_64-latest",
expectedCmd: []string{"gitlab-runner-helper", "health-check"},
......
......@@ -21,6 +21,7 @@ import (
"gitlab.com/gitlab-org/gitlab-runner/common"
"gitlab.com/gitlab-org/gitlab-runner/executors"
"gitlab.com/gitlab-org/gitlab-runner/helpers/dns"
"gitlab.com/gitlab-org/gitlab-runner/helpers/featureflags"
terminalsession "gitlab.com/gitlab-org/gitlab-runner/session/terminal"
)
......@@ -219,7 +220,7 @@ func (s *executor) buildContainer(name, image string, imageDefinition common.Ima
}
func (s *executor) getCommandAndArgs(imageDefinition common.Image, command ...string) ([]string, []string) {
if s.Build.IsFeatureFlagOn(common.FFK8sEntrypointOverCommand) {
if s.Build.IsFeatureFlagOn(featureflags.FFK8sEntrypointOverCommand) {
return s.getCommandsAndArgsV2(imageDefinition, command...)
}
......
......@@ -30,6 +30,7 @@ import (
"gitlab.com/gitlab-org/gitlab-runner/executors"
"gitlab.com/gitlab-org/gitlab-runner/helpers"
dns_test "gitlab.com/gitlab-org/gitlab-runner/helpers/dns/test"
"gitlab.com/gitlab-org/gitlab-runner/helpers/featureflags"
"gitlab.com/gitlab-org/gitlab-runner/session"
)
......@@ -1327,7 +1328,7 @@ func TestSetupBuildPod(t *testing.T) {
},
},
Variables: []common.JobVariable{
{Key: common.FFK8sEntrypointOverCommand, Value: "true"},
{Key: featureflags.FFK8sEntrypointOverCommand, Value: "true"},
},
Options: &kubernetesOptions{
Image: common.Image{
......@@ -1389,7 +1390,7 @@ func TestSetupBuildPod(t *testing.T) {
},
},
Variables: []common.JobVariable{
{Key: common.FFK8sEntrypointOverCommand, Value: "false"},
{Key: featureflags.FFK8sEntrypointOverCommand, Value: "false"},
},
Options: &kubernetesOptions{
Image: common.Image{
......
package featureflags
import (
"strconv"
)
const (
FFK8sEntrypointOverCommand string = "FF_K8S_USE_ENTRYPOINT_OVER_COMMAND"
FFDockerHelperImageV2 string = "FF_DOCKER_HELPER_IMAGE_V2"
FFCmdDisableDelayedErrorLevelExpansion string = "FF_CMD_DISABLE_DELAYED_ERROR_LEVEL_EXPANSION"
FFUseLegacyGitCleanStrategy string = "FF_USE_LEGACY_GIT_CLEAN_STRATEGY"
FFUseLegacyBuildsDirForDocker string = "FF_USE_LEGACY_BUILDS_DIR_FOR_DOCKER"
)
type FeatureFlag struct {
Name string
DefaultValue string
Deprecated bool
ToBeRemovedWith string
Description string
}
var flags = []FeatureFlag{
{
Name: FFK8sEntrypointOverCommand,
DefaultValue: "true",
Deprecated: true,
ToBeRemovedWith: "12.0",
Description: "Enables [the fix](https://gitlab.com/gitlab-org/gitlab-runner/merge_requests/1010) for entrypoint configuration when `kubernetes` executor is used",
},
{
Name: FFDockerHelperImageV2,
DefaultValue: "false",
Deprecated: true,
ToBeRemovedWith: "12.0",
Description: "Enable the helper image to use the new commands when [helper_image](https://docs.gitlab.com/runner/configuration/advanced-configuration.html#the-runnersdocker-section) is specified. This will start using the new API that will be used in 12.0 and stop showing the warning message in the build log",
},
{
Name: FFCmdDisableDelayedErrorLevelExpansion,
DefaultValue: "false",
Deprecated: false,
ToBeRemovedWith: "",
Description: "Disables [EnableDelayedExpansion](https://ss64.com/nt/delayedexpansion.html) for error checking for when using [Window Batch](https://docs.gitlab.com/runner/shells/#windows-batch) shell",
},
{
Name: FFUseLegacyGitCleanStrategy,
DefaultValue: "false",
Deprecated: true,
ToBeRemovedWith: "12.0",
Description: "Enables the new strategy for `git clean` that moves the clean operation after checkout and enables support for `GIT_CLEAN_FLAGS`",
},
{
Name: FFUseLegacyBuildsDirForDocker,
DefaultValue: "false",
Deprecated: true,
ToBeRemovedWith: "13.0",
Description: "Enables the new strategy for Docker executor to cache the content of `/builds` directory instead of `/builds/group-org`",
},
}
func GetAll() []FeatureFlag {
return flags
}
func IsOn(value string) (bool, error) {
if value == "" {
return false, nil
}
on, err := strconv.ParseBool(value)
if err != nil {
return false, err
}
return on, nil
}
package featureflags
import (
"testing"
"github.com/stretchr/testify/assert"
)
func mockFlags(newFlags ...FeatureFlag) func() {
oldFlags := flags
flags = newFlags
return func() {
flags = oldFlags
}
}
func TestGetAll(t *testing.T) {
testFlag := FeatureFlag{Name: "TEST_FLAG", DefaultValue: "value"}
defer mockFlags(testFlag)()
f := GetAll()
assert.Len(t, f, 1)
assert.Contains(t, f, testFlag)
}
func TestIsOn(t *testing.T) {
testCases := map[string]struct {
testValue string
expectedResult bool
expectedError string
}{
"empty value": {
testValue: "",
expectedResult: false,
},
"non boolean value": {
testValue: "a",
expectedResult: false,
expectedError: `strconv.ParseBool: parsing "a": invalid syntax`,
},
"true value": {
expectedResult: true,
testValue: "1",
},
"false value": {
expectedResult: false,
testValue: "f",
},
}
for testName, testCase := range testCases {
t.Run(testName, func(t *testing.T) {
result, err := IsOn(testCase.testValue)
assert.Equal(t, testCase.expectedResult, result)
if testCase.expectedError != "" {
assert.EqualError(t, err, testCase.expectedError)
} else {
assert.NoError(t, err)
}
})
}
}
......@@ -11,6 +11,7 @@ import (
"gitlab.com/gitlab-org/gitlab-runner/cache"
"gitlab.com/gitlab-org/gitlab-runner/common"
"gitlab.com/gitlab-org/gitlab-runner/helpers/featureflags"
"gitlab.com/gitlab-org/gitlab-runner/helpers/tls"
)
......@@ -96,7 +97,7 @@ func (b *AbstractShell) writeGitCleanup(w ShellWriter, build *common.Build) {
w.RmFile(".git/hooks/post-checkout")
// TODO: Remove in 12.0
if build.IsFeatureFlagOn(common.FFUseLegacyGitCleanStrategy) {
if build.IsFeatureFlagOn(featureflags.FFUseLegacyGitCleanStrategy) {
w.Command("git", "clean", "-ffdx")
w.IfCmd("git", "diff", "--no-ext-diff", "--quiet", "--exit-code")
// git 1.7 cannot reset before a checkout, if no diffs we can avoid git reset
......@@ -186,7 +187,7 @@ func (b *AbstractShell) writeCheckoutCmd(w ShellWriter, build *common.Build) {
w.Notice("Checking out %s as %s...", build.GitInfo.Sha[0:8], build.GitInfo.Ref)
w.Command("git", "checkout", "-f", "-q", build.GitInfo.Sha)
if !build.IsFeatureFlagOn(common.FFUseLegacyGitCleanStrategy) {
if !build.IsFeatureFlagOn(featureflags.FFUseLegacyGitCleanStrategy) {
cleanFlags := build.GetGitCleanFlags()
if len(cleanFlags) > 0 {
cleanArgs := append([]string{"clean"}, cleanFlags...)
......
......@@ -8,6 +8,7 @@ import (
"github.com/stretchr/testify/require"
"gitlab.com/gitlab-org/gitlab-runner/common"
"gitlab.com/gitlab-org/gitlab-runner/helpers/featureflags"
"gitlab.com/gitlab-org/gitlab-runner/helpers/tls"
)
......@@ -245,7 +246,7 @@ func TestGitCleanFlags(t *testing.T) {
GitInfo: common.GitInfo{Sha: dummySha, Ref: dummyRef},
Variables: common.JobVariables{
{Key: "GIT_CLEAN_FLAGS", Value: test.value},
{Key: common.FFUseLegacyGitCleanStrategy, Value: test.legacyCleanStrategy},
{Key: featureflags.FFUseLegacyGitCleanStrategy, Value: test.legacyCleanStrategy},
},
},
}
......
......@@ -12,6 +12,7 @@ import (
"gitlab.com/gitlab-org/gitlab-runner/common"
"gitlab.com/gitlab-org/gitlab-runner/helpers"
"gitlab.com/gitlab-org/gitlab-runner/helpers/featureflags"
)
type CmdShell struct {
......@@ -256,7 +257,7 @@ func (b *CmdShell) GetConfiguration(info common.ShellScriptInfo) (script *common
func (b *CmdShell) GenerateScript(buildStage common.BuildStage, info common.ShellScriptInfo) (script string, err error) {
w := &CmdWriter{
TemporaryPath: info.Build.TmpProjectDir(),
disableDelayedErrorLevelExpansion: info.Build.IsFeatureFlagOn(common.FFCmdDisableDelayedErrorLevelExpansion),
disableDelayedErrorLevelExpansion: info.Build.IsFeatureFlagOn(featureflags.FFCmdDisableDelayedErrorLevelExpansion),
}
if buildStage == common.BuildStagePrepare {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment