Skip to content

Commit

Permalink
refactor: remove container registry config (#42)
Browse files Browse the repository at this point in the history
For some reason we put the container registry config into the provider
configuration (i think there was some max length validation in the past
on garm side), whereas the tag of the image was definded in the ImageCR
respective Pool Entity in GARM.

This should now make ops and development easer, as one can define the
desired image tag of the runner pod as a whole string like:

```yaml
apiVersion: garm-operator.mercedes-benz.com/v1alpha1
kind: Image
metadata:
  labels:
    app.kubernetes.io/instance: image-sample
    app.kubernetes.io/name: image
    app.kubernetes.io/part-of: garm-operator
  name: runner-default
  namespace: garm-operator-system
spec:
  tag: localhost:5000/runner-default:main-49fe84a
```
  • Loading branch information
rafalgalaw authored May 7, 2024
1 parent 5a8b67e commit b74417a
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 67 deletions.
3 changes: 1 addition & 2 deletions internal/provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ type Provider struct {
func (p Provider) CreateInstance(_ context.Context, bootstrapParams params.BootstrapInstance) (params.ProviderInstance, error) {
podName := strings.ToLower(bootstrapParams.Name)
labels := spec.ParamsToPodLabels(p.ControllerID, bootstrapParams)
fullImageName := spec.GetFullImagePath(config.Config.ContainerRegistry, bootstrapParams.Image)
resourceRequirements := spec.FlavorToResourceRequirements(bootstrapParams.Flavor)

gitHubScopeDetails, err := spec.ExtractGitHubScopeDetails(bootstrapParams.RepoURL)
Expand All @@ -56,7 +55,7 @@ func (p Provider) CreateInstance(_ context.Context, bootstrapParams params.Boots
Containers: []corev1.Container{
{
Name: "runner",
Image: fullImageName,
Image: bootstrapParams.Image,
Resources: resourceRequirements,
Env: envs,
ImagePullPolicy: corev1.PullAlways,
Expand Down
50 changes: 21 additions & 29 deletions internal/provider/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,8 @@ func TestCreateInstance(t *testing.T) {
{
name: "Valid bootstrapParams and merge pod template spec",
config: &config.ProviderConfig{
KubeConfigPath: "",
ContainerRegistry: "localhost:5000",
RunnerNamespace: "runner",
KubeConfigPath: "",
RunnerNamespace: "runner",
PodTemplate: corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
Volumes: []corev1.Volume{
Expand Down Expand Up @@ -99,7 +98,7 @@ func TestCreateInstance(t *testing.T) {
InstanceToken: "test-token",
MetadataURL: "https://metadata.test",
CallbackURL: "https://callback.test/status",
Image: "runner:ubuntu-22.04",
Image: "localhost:5000/runner:ubuntu-22.04",
OSType: "linux",
OSArch: "arm64",
Labels: []string{"road-runner", "linux", "arm64", "kubernetes"},
Expand Down Expand Up @@ -258,9 +257,8 @@ func TestCreateInstance(t *testing.T) {
{
name: "Valid bootstrapParams and merge pod template spec with added sidecar",
config: &config.ProviderConfig{
KubeConfigPath: "",
ContainerRegistry: "localhost:5000",
RunnerNamespace: "runner",
KubeConfigPath: "",
RunnerNamespace: "runner",
PodTemplate: corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
Containers: []corev1.Container{
Expand Down Expand Up @@ -305,7 +303,7 @@ func TestCreateInstance(t *testing.T) {
InstanceToken: "test-token",
MetadataURL: "https://metadata.test",
CallbackURL: "https://callback.test/status",
Image: "runner:ubuntu-22.04",
Image: "localhost:5000/runner:ubuntu-22.04",
OSType: "linux",
OSArch: "arm64",
Labels: []string{"road-runner", "linux", "arm64", "kubernetes"},
Expand Down Expand Up @@ -454,9 +452,8 @@ func TestCreateInstance(t *testing.T) {
{
name: "Valid bootstrapParams no pod template spec",
config: &config.ProviderConfig{
KubeConfigPath: "",
ContainerRegistry: "localhost:5000",
RunnerNamespace: "runner",
KubeConfigPath: "",
RunnerNamespace: "runner",
PodTemplate: corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
Containers: []corev1.Container{},
Expand All @@ -471,7 +468,7 @@ func TestCreateInstance(t *testing.T) {
InstanceToken: "test-token",
MetadataURL: "https://metadata.test",
CallbackURL: "https://callback.test/status",
Image: "runner:ubuntu-22.04",
Image: "localhost:5000/runner:ubuntu-22.04",
OSType: "linux",
OSArch: "arm64",
Labels: []string{"road-runner", "linux", "arm64", "kubernetes"},
Expand Down Expand Up @@ -597,9 +594,8 @@ func TestCreateInstance(t *testing.T) {
{
name: "Valid bootstrapParams and merge pod template spec with livenessProbe",
config: &config.ProviderConfig{
KubeConfigPath: "",
ContainerRegistry: "localhost:5000",
RunnerNamespace: "runner",
KubeConfigPath: "",
RunnerNamespace: "runner",
PodTemplate: corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
Containers: []corev1.Container{
Expand Down Expand Up @@ -633,7 +629,7 @@ func TestCreateInstance(t *testing.T) {
InstanceToken: "test-token",
MetadataURL: "https://metadata.test",
CallbackURL: "https://callback.test/status",
Image: "runner:ubuntu-22.04",
Image: "localhost:5000/runner:ubuntu-22.04",
OSType: "linux",
OSArch: "arm64",
Labels: []string{"road-runner", "linux", "arm64", "kubernetes"},
Expand Down Expand Up @@ -775,9 +771,8 @@ func TestCreateInstance(t *testing.T) {
{
name: "Valid bootstrapParams and custom flavors",
config: &config.ProviderConfig{
KubeConfigPath: "",
ContainerRegistry: "localhost:5000",
RunnerNamespace: "runner",
KubeConfigPath: "",
RunnerNamespace: "runner",
PodTemplate: corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
Containers: []corev1.Container{
Expand Down Expand Up @@ -831,7 +826,7 @@ func TestCreateInstance(t *testing.T) {
InstanceToken: "test-token",
MetadataURL: "https://metadata.test",
CallbackURL: "https://callback.test/status",
Image: "runner:ubuntu-22.04",
Image: "localhost:5000/runner:ubuntu-22.04",
OSType: "linux",
OSArch: "arm64",
Labels: []string{"road-runner", "linux", "arm64", "kubernetes"},
Expand Down Expand Up @@ -1019,9 +1014,8 @@ func TestGetInstance(t *testing.T) {
{
name: "Get Instance",
config: &config.ProviderConfig{
KubeConfigPath: "",
ContainerRegistry: "localhost:5000",
RunnerNamespace: "runner",
KubeConfigPath: "",
RunnerNamespace: "runner",
},
expectedProviderInstance: params.ProviderInstance{
ProviderID: providerID,
Expand Down Expand Up @@ -1176,9 +1170,8 @@ func TestDeleteInstance(t *testing.T) {
{
name: "Delete Instance Success",
config: &config.ProviderConfig{
KubeConfigPath: "",
ContainerRegistry: "localhost:5000",
RunnerNamespace: "runner",
KubeConfigPath: "",
RunnerNamespace: "runner",
},
runtimeObjects: []runtime.Object{
&corev1.Pod{
Expand Down Expand Up @@ -1353,9 +1346,8 @@ func TestRemoveAllInstances(t *testing.T) {
{
name: "Remove All Instances Success",
config: &config.ProviderConfig{
KubeConfigPath: "",
ContainerRegistry: "localhost:5000",
RunnerNamespace: "runner",
KubeConfigPath: "",
RunnerNamespace: "runner",
},
runtimeObjects: []runtime.Object{
&corev1.Pod{
Expand Down
6 changes: 0 additions & 6 deletions internal/spec/spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package spec
import (
"fmt"
"net/url"
"path/filepath"
"strings"
"unicode"

Expand Down Expand Up @@ -278,8 +277,3 @@ func ExtractImageDetails(pod *corev1.Pod) *ImageDetails {
OSArch: OSArch(pod.Labels[GarmOSArchLabel]),
}
}

func GetFullImagePath(containerRegistry, imageNameAndTag string) string {
reg := filepath.Clean(containerRegistry)
return reg + "/" + imageNameAndTag
}
9 changes: 4 additions & 5 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,10 @@ import (
)

type ProviderConfig struct {
KubeConfigPath string `koanf:"kubeConfigPath"`
ContainerRegistry string `koanf:"containerRegistry"`
RunnerNamespace string `koanf:"runnerNamespace"`
PodTemplate corev1.PodTemplateSpec `koanf:"podTemplate"`
Flavors map[string]corev1.ResourceRequirements `koanf:"flavors"`
KubeConfigPath string `koanf:"kubeConfigPath"`
RunnerNamespace string `koanf:"runnerNamespace"`
PodTemplate corev1.PodTemplateSpec `koanf:"podTemplate"`
Flavors map[string]corev1.ResourceRequirements `koanf:"flavors"`
}

var Config ProviderConfig
Expand Down
37 changes: 12 additions & 25 deletions pkg/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,8 @@ func TestGetConfig(t *testing.T) {
{
name: "valid configuration withouth PodTemplateSpec",
expected: config.ProviderConfig{
KubeConfigPath: "/path/to/kubeconfig",
ContainerRegistry: "sample.registry.com",
RunnerNamespace: "test-namespace",
KubeConfigPath: "/path/to/kubeconfig",
RunnerNamespace: "test-namespace",
PodTemplate: corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
Containers: []corev1.Container{},
Expand All @@ -35,16 +34,14 @@ func TestGetConfig(t *testing.T) {
},
config: `
kubeConfigPath: "/path/to/kubeconfig"
containerRegistry: "sample.registry.com"
runnerNamespace: "test-namespace"
`,
},
{
name: "valid configuration - expect default namespace",
expected: config.ProviderConfig{
KubeConfigPath: "/path/to/kubeconfig",
ContainerRegistry: "sample.registry.com",
RunnerNamespace: "runner",
KubeConfigPath: "/path/to/kubeconfig",
RunnerNamespace: "runner",
PodTemplate: corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
Containers: []corev1.Container{},
Expand All @@ -53,15 +50,13 @@ runnerNamespace: "test-namespace"
},
config: `
kubeConfigPath: "/path/to/kubeconfig"
containerRegistry: "sample.registry.com"
`,
},
{
name: "valid configuration without a defined registry is fine - using configured container runtime registries",
expected: config.ProviderConfig{
KubeConfigPath: "/path/to/kubeconfig",
ContainerRegistry: "",
RunnerNamespace: "runner",
KubeConfigPath: "/path/to/kubeconfig",
RunnerNamespace: "runner",
PodTemplate: corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
Containers: []corev1.Container{},
Expand All @@ -70,16 +65,14 @@ containerRegistry: "sample.registry.com"
},
config: `
kubeConfigPath: "/path/to/kubeconfig"
containerRegistry: ""
runnerNamespace: "runner"
`,
},
{
name: "invalid configuration with a invalid namespace name",
expected: config.ProviderConfig{
KubeConfigPath: "/path/to/kubeconfig",
ContainerRegistry: "",
RunnerNamespace: "runner",
KubeConfigPath: "/path/to/kubeconfig",
RunnerNamespace: "runner",
PodTemplate: corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
Containers: []corev1.Container{},
Expand All @@ -88,17 +81,15 @@ runnerNamespace: "runner"
},
config: `
kubeConfigPath: "/path/to/kubeconfig"
containerRegistry: ""
runnerNamespace: "this_is_An_invalid_namespace_name"
`,
wantError: true,
},
{
name: "valid configuration with custom flavor to resource requirements mapping",
expected: config.ProviderConfig{
KubeConfigPath: "/path/to/kubeconfig",
ContainerRegistry: "",
RunnerNamespace: "runner",
KubeConfigPath: "/path/to/kubeconfig",
RunnerNamespace: "runner",
PodTemplate: corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
Containers: []corev1.Container{},
Expand Down Expand Up @@ -127,7 +118,6 @@ runnerNamespace: "this_is_An_invalid_namespace_name"
},
config: `
kubeConfigPath: "/path/to/kubeconfig"
containerRegistry: ""
runnerNamespace: "runner"
flavors:
micro:
Expand All @@ -148,9 +138,8 @@ flavors:
{
name: "valid configuration with extra livenessProbe",
expected: config.ProviderConfig{
KubeConfigPath: "/path/to/kubeconfig",
ContainerRegistry: "",
RunnerNamespace: "runner",
KubeConfigPath: "/path/to/kubeconfig",
RunnerNamespace: "runner",
PodTemplate: corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
Containers: []corev1.Container{
Expand All @@ -177,7 +166,6 @@ flavors:
},
config: `
kubeConfigPath: "/path/to/kubeconfig"
containerRegistry: ""
runnerNamespace: "runner"
podTemplate:
spec:
Expand Down Expand Up @@ -213,7 +201,6 @@ podTemplate:

if tc.wantError == false && err == nil {
assert.Equal(t, tc.expected.KubeConfigPath, config.Config.KubeConfigPath)
assert.Equal(t, tc.expected.ContainerRegistry, config.Config.ContainerRegistry)
assert.Equal(t, tc.expected.RunnerNamespace, config.Config.RunnerNamespace)
assert.Equal(t, tc.expected.PodTemplate, config.Config.PodTemplate)
assert.Equal(t, tc.expected.Flavors, config.Config.Flavors)
Expand Down

0 comments on commit b74417a

Please sign in to comment.