diff options
author | John Cai <jcai@gitlab.com> | 2023-08-23 17:01:05 +0300 |
---|---|---|
committer | John Cai <jcai@gitlab.com> | 2023-08-23 18:23:18 +0300 |
commit | 65af112b84cfaee958b2e4cb83ac8fbb323ff246 (patch) | |
tree | 900c5cb342fdf8b32953844d5bbb26828dcd9f52 | |
parent | 5649e8da1e31fcaa495da9420a56da09b61236f6 (diff) |
command: Remove RunCmdInCgroup feature flag
Cgroups have been running production by default for a long time now.
Let's remove the feature flag completely.
-rw-r--r-- | internal/command/command.go | 2 | ||||
-rw-r--r-- | internal/featureflag/ff_run_cmd_in_cgroup.go | 9 | ||||
-rw-r--r-- | internal/git/command_factory_cgroup_test.go | 48 | ||||
-rw-r--r-- | internal/testhelper/testhelper.go | 2 |
4 files changed, 14 insertions, 47 deletions
diff --git a/internal/command/command.go b/internal/command/command.go index 6ccc25adc..6f6d9738c 100644 --- a/internal/command/command.go +++ b/internal/command/command.go @@ -329,7 +329,7 @@ func New(ctx context.Context, nameAndArgs []string, opts ...Option) (*Command, e }() }() - if featureflag.RunCommandsInCGroup.IsEnabled(ctx) && cfg.cgroupsManager != nil { + if cfg.cgroupsManager != nil { cgroupPath, err := cfg.cgroupsManager.AddCommand(command.cmd, cfg.cgroupsAddCommandOpts...) if err != nil { return nil, err diff --git a/internal/featureflag/ff_run_cmd_in_cgroup.go b/internal/featureflag/ff_run_cmd_in_cgroup.go deleted file mode 100644 index 356bae752..000000000 --- a/internal/featureflag/ff_run_cmd_in_cgroup.go +++ /dev/null @@ -1,9 +0,0 @@ -package featureflag - -// RunCommandsInCGroup allows all commands to be run within a cgroup -var RunCommandsInCGroup = NewFeatureFlag( - "run_cmds_in_cgroup", - "v14.10.0", - "https://gitlab.com/gitlab-org/gitaly/-/issues/4102", - true, -) diff --git a/internal/git/command_factory_cgroup_test.go b/internal/git/command_factory_cgroup_test.go index 39404db23..6b92bf4b4 100644 --- a/internal/git/command_factory_cgroup_test.go +++ b/internal/git/command_factory_cgroup_test.go @@ -6,7 +6,6 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/v16/internal/cgroups" - "gitlab.com/gitlab-org/gitaly/v16/internal/featureflag" "gitlab.com/gitlab-org/gitaly/v16/internal/git" "gitlab.com/gitlab-org/gitaly/v16/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/v16/internal/testhelper" @@ -33,41 +32,18 @@ func TestNewCommandAddsToCgroup(t *testing.T) { SkipCreationViaService: true, }) - for _, tc := range []struct { - desc string - cgroupsFF bool - }{ - { - desc: "cgroups feature flag on", - cgroupsFF: true, - }, - { - desc: "cgroups feature flag off", - cgroupsFF: false, - }, - } { - t.Run(tc.desc, func(t *testing.T) { - ctx := featureflag.IncomingCtxWithFeatureFlag(ctx, featureflag.RunCommandsInCGroup, tc.cgroupsFF) - - var manager mockCgroupsManager - gitCmdFactory := gittest.NewCommandFactory(t, cfg, git.WithCgroupsManager(&manager)) + var manager mockCgroupsManager + gitCmdFactory := gittest.NewCommandFactory(t, cfg, git.WithCgroupsManager(&manager)) - cmd, err := gitCmdFactory.New(ctx, repo, git.Command{ - Name: "rev-parse", - Flags: []git.Option{ - git.Flag{Name: "--is-bare-repository"}, - }, - }) - require.NoError(t, err) - require.NoError(t, cmd.Wait()) - - if tc.cgroupsFF { - require.Len(t, manager.commands, 1) - require.Contains(t, manager.commands[0].Args, "rev-parse") - return - } + cmd, err := gitCmdFactory.New(ctx, repo, git.Command{ + Name: "rev-parse", + Flags: []git.Option{ + git.Flag{Name: "--is-bare-repository"}, + }, + }) + require.NoError(t, err) + require.NoError(t, cmd.Wait()) - require.Len(t, manager.commands, 0) - }) - } + require.Len(t, manager.commands, 1) + require.Contains(t, manager.commands[0].Args, "rev-parse") } diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go index 897afe429..d588b23fa 100644 --- a/internal/testhelper/testhelper.go +++ b/internal/testhelper/testhelper.go @@ -235,7 +235,7 @@ func ContextWithoutCancel(opts ...ContextOpt) context.Context { // There are some feature flags we need to enable in this function because they end up very // deep in the call stack, so almost every test function would have to inject it into its // context. The values of these flags should be randomized to increase the test coverage. - ctx = featureflag.ContextWithFeatureFlag(ctx, featureflag.RunCommandsInCGroup, true) + // Randomly enable mailmap ctx = featureflag.ContextWithFeatureFlag(ctx, featureflag.MailmapOptions, rand.Int()%2 == 0) |