Commit f0df90c8 authored by Kamil Trzciński's avatar Kamil Trzciński

Merge branch 'fix-kubernetes-support-for-extended-docker-configuration' into 'master'

Fix command and args assignment when creating containers with K8S executor

Closes #2338 and #3536

See merge request !1010
parents 5e01a1aa 3a048cd1
Pipeline #31764227 passed with stages
in 30 minutes and 17 seconds
......@@ -463,6 +463,12 @@ func (b *Build) GetDefaultVariables() JobVariables {
}
}
func (b *Build) GetDefaultFeatureFlagsVariables() JobVariables {
return JobVariables{
{Key: "FF_K8S_USE_ENTRYPOINT_OVER_COMMAND", Value: "true", Public: true, Internal: true, File: false}, // TODO: Remove in 12.0
}
}
func (b *Build) GetSharedEnvVariable() JobVariable {
env := JobVariable{Value: "true", Public: true, Internal: true, File: false}
if b.IsSharedEnv() {
......@@ -512,6 +518,7 @@ func (b *Build) GetAllVariables() JobVariables {
variables = append(variables, b.Runner.GetVariables()...)
}
variables = append(variables, b.GetDefaultVariables()...)
variables = append(variables, b.GetDefaultFeatureFlagsVariables()...)
variables = append(variables, b.GetCITLSVariables()...)
variables = append(variables, b.Variables...)
variables = append(variables, b.GetSharedEnvVariable())
......@@ -660,3 +667,23 @@ func NewBuild(jobData JobResponse, runnerConfig *RunnerConfig, systemInterrupt c
createdAt: time.Now(),
}
}
func (b *Build) IsFeatureFlagOn(name string) bool {
ffValue := b.GetAllVariables().Get(name)
if ffValue == "" {
return false
}
on, err := strconv.ParseBool(ffValue)
if err != nil {
logrus.WithError(err).
WithField("ffName", name).
WithField("ffValue", ffValue).
Error("Error while parsing the value of feature flag")
return false
}
return on
}
package common
import (
"errors"
"fmt"
"os"
"testing"
"errors"
"github.com/sirupsen/logrus/hooks/test"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
)
func init() {
......@@ -590,3 +591,73 @@ func TestGetRemoteURL(t *testing.T) {
assert.Equal(t, tc.result, build.GetRemoteURL())
}
}
type featureFlagOnTestCase struct {
value string
expectedStatus bool
expectedError bool
}
func TestIsFeatureFlagOn(t *testing.T) {
hook := test.NewGlobal()
tests := map[string]featureFlagOnTestCase{
"no value": {
value: "",
expectedStatus: false,
expectedError: false,
},
"true": {
value: "true",
expectedStatus: true,
expectedError: false,
},
"1": {
value: "1",
expectedStatus: true,
expectedError: false,
},
"false": {
value: "false",
expectedStatus: false,
expectedError: false,
},
"0": {
value: "0",
expectedStatus: false,
expectedError: false,
},
"invalid value": {
value: "test",
expectedStatus: false,
expectedError: true,
},
}
for name, testCase := range tests {
t.Run(name, func(t *testing.T) {
build := new(Build)
build.Variables = JobVariables{
{Key: "FF_TEST_FEATURE", Value: testCase.value},
}
status := build.IsFeatureFlagOn("FF_TEST_FEATURE")
assert.Equal(t, testCase.expectedStatus, status)
entry := hook.LastEntry()
if testCase.expectedError {
require.NotNil(t, entry)
logrusOutput, err := entry.String()
require.NoError(t, err)
assert.Contains(t, logrusOutput, "Error while parsing the value of feature flag")
} else {
assert.Nil(t, entry)
}
hook.Reset()
})
}
}
......@@ -5,6 +5,7 @@ import (
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestVariablesJSON(t *testing.T) {
......@@ -93,3 +94,60 @@ func TestSpecialVariablesExpansion(t *testing.T) {
assert.Equal(t, "aabb", expanded.Get("key3"))
assert.Equal(t, "aabb", expanded.Get("key4"))
}
type multipleKeyUsagesTestCase struct {
variables JobVariables
expectedValue string
}
func TestMultipleUsageOfAKey(t *testing.T) {
getVariable := func(value string) JobVariable {
return JobVariable{Key: "key", Value: value}
}
tests := map[string]multipleKeyUsagesTestCase{
"defined at job level": {
variables: JobVariables{
getVariable("from-job"),
},
expectedValue: "from-job",
},
"defined at default and job level": {
variables: JobVariables{
getVariable("from-default"),
getVariable("from-job"),
},
expectedValue: "from-job",
},
"defined at config, default and job level": {
variables: JobVariables{
getVariable("from-config"),
getVariable("from-default"),
getVariable("from-job"),
},
expectedValue: "from-job",
},
"defined at config and default level": {
variables: JobVariables{
getVariable("from-config"),
getVariable("from-default"),
},
expectedValue: "from-default",
},
"defined at config level": {
variables: JobVariables{
getVariable("from-config"),
},
expectedValue: "from-config",
},
}
for name, testCase := range tests {
t.Run(name, func(t *testing.T) {
for i := 0; i < 100; i++ {
require.Equal(t, testCase.expectedValue, testCase.variables.Get("key"))
}
})
}
}
# Feature flags
Starting with GitLab Runner 11.4 we added a base support for feature flags in GitLab Runner.
These flags may be used:
1. For beta features that we want to make available for volunteers but don't want to enable publicly yet.
A user who wants to use such feature and accepts the risk, can enable it explicitly while the wide
group of users will be unaffected by potential bugs and regressions.
1. For breaking changes that need a deprecation and removal after few releases.
While the product evolves some features may be removed or changed. Sometimes it may be even something
that is generally considered as a bug, but users already managed to find some workarounds for it
and a fix could affect their configurations.
In that cases the feature flag is used to switch from old behavior to the new wan on demand. Such
fix such ensure that the old behavior is deprecated and marked for removal together with the feature
flag that protects the new behavior.
At this moment feature flags mechanism is based on environment variables. To make the change hidden behind
the feature flag active a corresponding environment variable should be set to `true` or `1`. To make the
change hidden behind the feature flag disabled a corresponding environment variable should be set to
`false` or `0`.
## Available feature flags
| Feature flag | Default value | Deprecated | To be removed with | Description |
|--------------------------------------|---------------|------------|--------------------|-------------|
| `FF_K8S_USE_ENTRYPOINT_OVER_COMMAND` | `true` | ✓ | 12.0 | Enables [the fix][mr-1010] for entrypoint configuration when `kubernetes` executor is used. |
[mr-1010]: https://gitlab.com/gitlab-org/gitlab-runner/merge_requests/1010
......@@ -123,6 +123,7 @@ To jump into the specific documentation of each executor, visit:
- [Runner monitoring](monitoring/README.md) Learn how to monitor the Runner's behavior.
- [Cleanup the Docker images automatically](https://gitlab.com/gitlab-org/gitlab-runner-docker-cleanup) A simple Docker application that automatically garbage collects the GitLab Runner caches and images when running low on disk space.
- [Configure GitLab Runner to run behind a proxy](configuration/proxy.md) Learn how to set up a Linux proxy and configure GitLab Runner. Especially useful for the Docker executor.
- [Feature Flags](configuration/feature-flags.md) Learn how to use feature flags to get access to features in beta stage or to enable breaking changes before the full deprecation and replacement is handled.
## Troubleshooting
......
......@@ -187,21 +187,13 @@ func (s *executor) Cleanup() {
s.AbstractExecutor.Cleanup()
}
func (s *executor) buildContainer(name, image string, imageDefinition common.Image, requests, limits api.ResourceList, command ...string) api.Container {
func (s *executor) buildContainer(name, image string, imageDefinition common.Image, requests, limits api.ResourceList, containerCommand ...string) api.Container {
privileged := false
if s.Config.Kubernetes != nil {
privileged = s.Config.Kubernetes.Privileged
}
if len(command) == 0 && len(imageDefinition.Command) > 0 {
command = imageDefinition.Command
}
var args []string
if len(imageDefinition.Entrypoint) > 0 {
args = command
command = imageDefinition.Entrypoint
}
command, args := s.getCommandAndArgs(imageDefinition, containerCommand...)
return api.Container{
Name: name,
......@@ -222,6 +214,43 @@ 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("FF_K8S_USE_ENTRYPOINT_OVER_COMMAND") {
return s.getCommandsAndArgsV2(imageDefinition, command...)
}
return s.getCommandsAndArgsV1(imageDefinition, command...)
}
// TODO: Remove in 12.0
func (s *executor) getCommandsAndArgsV1(imageDefinition common.Image, command ...string) ([]string, []string) {
if len(command) == 0 && len(imageDefinition.Command) > 0 {
command = imageDefinition.Command
}
var args []string
if len(imageDefinition.Entrypoint) > 0 {
args = command
command = imageDefinition.Entrypoint
}
return command, args
}
// TODO: Make this the only proper way to setup command and args in 12.0
func (s *executor) getCommandsAndArgsV2(imageDefinition common.Image, command ...string) ([]string, []string) {
if len(command) == 0 && len(imageDefinition.Entrypoint) > 0 {
command = imageDefinition.Entrypoint
}
var args []string
if len(imageDefinition.Command) > 0 {
args = imageDefinition.Command
}
return command, args
}
func (s *executor) getVolumeMounts() (mounts []api.VolumeMount) {
path := strings.Split(s.Build.BuildDir, "/")
path = path[:len(path)-1]
......
......@@ -1214,10 +1214,14 @@ func TestSetupBuildPod(t *testing.T) {
Entrypoint: []string{"/init", "run"},
Command: []string{"application", "--debug"},
},
{
Name: "test-service-2",
Command: []string{"application", "--debug"},
},
},
},
VerifyFn: func(t *testing.T, test setupBuildPodTestDef, pod *api.Pod) {
require.Len(t, pod.Spec.Containers, 3)
require.Len(t, pod.Spec.Containers, 4)
assert.Equal(t, "build", pod.Spec.Containers[0].Name)
assert.Equal(t, "test-image", pod.Spec.Containers[0].Image)
......@@ -1233,6 +1237,135 @@ func TestSetupBuildPod(t *testing.T) {
assert.Equal(t, "test-service", pod.Spec.Containers[2].Image)
assert.Equal(t, []string{"/init", "run"}, pod.Spec.Containers[2].Command)
assert.Equal(t, []string{"application", "--debug"}, pod.Spec.Containers[2].Args)
assert.Equal(t, "svc-1", pod.Spec.Containers[3].Name)
assert.Equal(t, "test-service-2", pod.Spec.Containers[3].Image)
assert.Empty(t, pod.Spec.Containers[3].Command, "Service container command should be empty")
assert.Equal(t, []string{"application", "--debug"}, pod.Spec.Containers[3].Args)
},
},
// TODO: Remove the mention of Feature Flag in 12.0, make it the only proper test case.
"properly sets command (entrypoint) and args when FF_K8S_USE_ENTRYPOINT_OVER_COMMAND is on": {
RunnerConfig: common.RunnerConfig{
RunnerSettings: common.RunnerSettings{
Kubernetes: &common.KubernetesConfig{
Namespace: "default",
HelperImage: "custom/helper-image",
},
},
},
Variables: []common.JobVariable{
{Key: "FF_K8S_USE_ENTRYPOINT_OVER_COMMAND", Value: "true"},
},
Options: &kubernetesOptions{
Image: common.Image{
Name: "test-image",
},
Services: common.Services{
{
Name: "test-service-0",
Command: []string{"application", "--debug"},
},
{
Name: "test-service-1",
Entrypoint: []string{"application", "--debug"},
},
{
Name: "test-service-2",
Entrypoint: []string{"application", "--debug"},
Command: []string{"argument1", "argument2"},
},
},
},
VerifyFn: func(t *testing.T, test setupBuildPodTestDef, pod *api.Pod) {
require.Len(t, pod.Spec.Containers, 5)
assert.Equal(t, "build", pod.Spec.Containers[0].Name)
assert.Equal(t, "test-image", pod.Spec.Containers[0].Image)
assert.Empty(t, pod.Spec.Containers[0].Command, "Build container command should be empty")
assert.Empty(t, pod.Spec.Containers[0].Args, "Build container args should be empty")
assert.Equal(t, "helper", pod.Spec.Containers[1].Name)
assert.Equal(t, "custom/helper-image", pod.Spec.Containers[1].Image)
assert.Empty(t, pod.Spec.Containers[1].Command, "Helper container command should be empty")
assert.Empty(t, pod.Spec.Containers[1].Args, "Helper container args should be empty")
assert.Equal(t, "svc-0", pod.Spec.Containers[2].Name)
assert.Equal(t, "test-service-0", pod.Spec.Containers[2].Image)
assert.Empty(t, pod.Spec.Containers[2].Command, "Service container command should be empty")
assert.Equal(t, []string{"application", "--debug"}, pod.Spec.Containers[2].Args)
assert.Equal(t, "svc-1", pod.Spec.Containers[3].Name)
assert.Equal(t, "test-service-1", pod.Spec.Containers[3].Image)
assert.Equal(t, []string{"application", "--debug"}, pod.Spec.Containers[3].Command)
assert.Empty(t, pod.Spec.Containers[3].Args, "Service container args should be empty")
assert.Equal(t, "svc-2", pod.Spec.Containers[4].Name)
assert.Equal(t, "test-service-2", pod.Spec.Containers[4].Image)
assert.Equal(t, []string{"application", "--debug"}, pod.Spec.Containers[4].Command)
assert.Equal(t, []string{"argument1", "argument2"}, pod.Spec.Containers[4].Args)
},
},
// TODO: Remove in 12.0
"sets command (entrypoint) and args in old way when FF_K8S_USE_ENTRYPOINT_OVER_COMMAND is off": {
RunnerConfig: common.RunnerConfig{
RunnerSettings: common.RunnerSettings{
Kubernetes: &common.KubernetesConfig{
Namespace: "default",
HelperImage: "custom/helper-image",
},
},
},
Variables: []common.JobVariable{
{Key: "FF_K8S_USE_ENTRYPOINT_OVER_COMMAND", Value: "false"},
},
Options: &kubernetesOptions{
Image: common.Image{
Name: "test-image",
},
Services: common.Services{
{
Name: "test-service-0",
Command: []string{"application", "--debug"},
},
{
Name: "test-service-1",
Entrypoint: []string{"application", "--debug"},
},
{
Name: "test-service-2",
Entrypoint: []string{"application", "--debug"},
Command: []string{"argument1", "argument2"},
},
},
},
VerifyFn: func(t *testing.T, test setupBuildPodTestDef, pod *api.Pod) {
require.Len(t, pod.Spec.Containers, 5)
assert.Equal(t, "build", pod.Spec.Containers[0].Name)
assert.Equal(t, "test-image", pod.Spec.Containers[0].Image)
assert.Empty(t, pod.Spec.Containers[0].Command, "Build container command should be empty")
assert.Empty(t, pod.Spec.Containers[0].Args, "Build container args should be empty")
assert.Equal(t, "helper", pod.Spec.Containers[1].Name)
assert.Equal(t, "custom/helper-image", pod.Spec.Containers[1].Image)
assert.Empty(t, pod.Spec.Containers[1].Command, "Helper container command should be empty")
assert.Empty(t, pod.Spec.Containers[1].Args, "Helper container args should be empty")
assert.Equal(t, "svc-0", pod.Spec.Containers[2].Name)
assert.Equal(t, "test-service-0", pod.Spec.Containers[2].Image)
assert.Equal(t, []string{"application", "--debug"}, pod.Spec.Containers[2].Command)
assert.Empty(t, pod.Spec.Containers[2].Args, "Service container command should be empty")
assert.Equal(t, "svc-1", pod.Spec.Containers[3].Name)
assert.Equal(t, "test-service-1", pod.Spec.Containers[3].Image)
assert.Equal(t, []string{"application", "--debug"}, pod.Spec.Containers[3].Command)
assert.Empty(t, pod.Spec.Containers[3].Args, "Service container args should be empty")
assert.Equal(t, "svc-2", pod.Spec.Containers[4].Name)
assert.Equal(t, "test-service-2", pod.Spec.Containers[4].Image)
assert.Equal(t, []string{"application", "--debug"}, pod.Spec.Containers[4].Command)
assert.Equal(t, []string{"argument1", "argument2"}, pod.Spec.Containers[4].Args)
},
},
}
......
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