Fix Sidecar without image specification issue (#2977)
Co-authored-by: Oleg Nozdrachev <ovnozdrach@mts.ru> Co-authored-by: Felix Kunde <felix-kunde@gmx.de>
This commit is contained in:
ovnozdrach
GitHub
parent
744372b09b
commit
42bbead4c9
|
|
@ -1303,6 +1303,9 @@ func (c *Cluster) generateStatefulSet(spec *acidv1.PostgresSpec) (*appsv1.Statef
|
|||
c.logger.Warningf("initContainers specified but disabled in configuration - next statefulset creation would fail")
|
||||
}
|
||||
initContainers = spec.InitContainers
|
||||
if err := c.validateContainers(initContainers); err != nil {
|
||||
return nil, fmt.Errorf("invalid init containers: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// backward compatible check for InitContainers
|
||||
|
|
@ -1455,6 +1458,10 @@ func (c *Cluster) generateStatefulSet(spec *acidv1.PostgresSpec) (*appsv1.Statef
|
|||
|
||||
sidecarContainers = patchSidecarContainers(sidecarContainers, volumeMounts, c.OpConfig.SuperUsername, c.credentialSecretName(c.OpConfig.SuperUsername))
|
||||
|
||||
if err := c.validateContainers(sidecarContainers); err != nil {
|
||||
return nil, fmt.Errorf("invalid sidecar containers: %v", err)
|
||||
}
|
||||
|
||||
tolerationSpec := tolerations(&spec.Tolerations, c.OpConfig.PodToleration)
|
||||
effectivePodPriorityClassName := util.Coalesce(spec.PodPriorityClassName, c.OpConfig.PodPriorityClassName)
|
||||
|
||||
|
|
@ -2592,3 +2599,15 @@ func ensurePath(file string, defaultDir string, defaultFile string) string {
|
|||
}
|
||||
return file
|
||||
}
|
||||
|
||||
func (c *Cluster) validateContainers(containers []v1.Container) error {
|
||||
for i, container := range containers {
|
||||
if container.Name == "" {
|
||||
return fmt.Errorf("container[%d]: name is required", i)
|
||||
}
|
||||
if container.Image == "" {
|
||||
return fmt.Errorf("container '%v': image is required", container.Name)
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1935,7 +1935,8 @@ func TestAdditionalVolume(t *testing.T) {
|
|||
AdditionalVolumes: additionalVolumes,
|
||||
Sidecars: []acidv1.Sidecar{
|
||||
{
|
||||
Name: sidecarName,
|
||||
Name: sidecarName,
|
||||
DockerImage: "test-image",
|
||||
},
|
||||
},
|
||||
},
|
||||
|
|
@ -2163,10 +2164,12 @@ func TestSidecars(t *testing.T) {
|
|||
},
|
||||
Sidecars: []acidv1.Sidecar{
|
||||
{
|
||||
Name: "cluster-specific-sidecar",
|
||||
Name: "cluster-specific-sidecar",
|
||||
DockerImage: "test-image",
|
||||
},
|
||||
{
|
||||
Name: "cluster-specific-sidecar-with-resources",
|
||||
Name: "cluster-specific-sidecar-with-resources",
|
||||
DockerImage: "test-image",
|
||||
Resources: &acidv1.Resources{
|
||||
ResourceRequests: acidv1.ResourceDescription{CPU: k8sutil.StringToPointer("210m"), Memory: k8sutil.StringToPointer("0.8Gi")},
|
||||
ResourceLimits: acidv1.ResourceDescription{CPU: k8sutil.StringToPointer("510m"), Memory: k8sutil.StringToPointer("1.4Gi")},
|
||||
|
|
@ -2201,7 +2204,8 @@ func TestSidecars(t *testing.T) {
|
|||
},
|
||||
SidecarContainers: []v1.Container{
|
||||
{
|
||||
Name: "global-sidecar",
|
||||
Name: "global-sidecar",
|
||||
Image: "test-image",
|
||||
},
|
||||
// will be replaced by a cluster specific sidecar with the same name
|
||||
{
|
||||
|
|
@ -2271,6 +2275,7 @@ func TestSidecars(t *testing.T) {
|
|||
// cluster specific sidecar
|
||||
assert.Contains(t, s.Spec.Template.Spec.Containers, v1.Container{
|
||||
Name: "cluster-specific-sidecar",
|
||||
Image: "test-image",
|
||||
Env: env,
|
||||
Resources: generateKubernetesResources("200m", "500m", "0.7Gi", "1.3Gi"),
|
||||
ImagePullPolicy: v1.PullIfNotPresent,
|
||||
|
|
@ -2297,6 +2302,7 @@ func TestSidecars(t *testing.T) {
|
|||
// global sidecar
|
||||
assert.Contains(t, s.Spec.Template.Spec.Containers, v1.Container{
|
||||
Name: "global-sidecar",
|
||||
Image: "test-image",
|
||||
Env: env,
|
||||
VolumeMounts: mounts,
|
||||
})
|
||||
|
|
@ -2325,6 +2331,180 @@ func TestSidecars(t *testing.T) {
|
|||
|
||||
}
|
||||
|
||||
func TestContainerValidation(t *testing.T) {
|
||||
testCases := []struct {
|
||||
name string
|
||||
spec acidv1.PostgresSpec
|
||||
clusterConfig Config
|
||||
expectedError string
|
||||
}{
|
||||
{
|
||||
name: "init container without image",
|
||||
spec: acidv1.PostgresSpec{
|
||||
PostgresqlParam: acidv1.PostgresqlParam{
|
||||
PgVersion: "17",
|
||||
},
|
||||
TeamID: "myapp",
|
||||
NumberOfInstances: 1,
|
||||
Volume: acidv1.Volume{
|
||||
Size: "1G",
|
||||
},
|
||||
InitContainers: []v1.Container{
|
||||
{
|
||||
Name: "invalid-initcontainer",
|
||||
},
|
||||
},
|
||||
},
|
||||
clusterConfig: Config{
|
||||
OpConfig: config.Config{
|
||||
PodManagementPolicy: "ordered_ready",
|
||||
ProtectedRoles: []string{"admin"},
|
||||
Auth: config.Auth{
|
||||
SuperUsername: superUserName,
|
||||
ReplicationUsername: replicationUserName,
|
||||
},
|
||||
},
|
||||
},
|
||||
expectedError: "image is required",
|
||||
},
|
||||
{
|
||||
name: "sidecar without name",
|
||||
spec: acidv1.PostgresSpec{
|
||||
PostgresqlParam: acidv1.PostgresqlParam{
|
||||
PgVersion: "17",
|
||||
},
|
||||
TeamID: "myapp",
|
||||
NumberOfInstances: 1,
|
||||
Volume: acidv1.Volume{
|
||||
Size: "1G",
|
||||
},
|
||||
},
|
||||
clusterConfig: Config{
|
||||
OpConfig: config.Config{
|
||||
PodManagementPolicy: "ordered_ready",
|
||||
ProtectedRoles: []string{"admin"},
|
||||
Auth: config.Auth{
|
||||
SuperUsername: superUserName,
|
||||
ReplicationUsername: replicationUserName,
|
||||
},
|
||||
SidecarContainers: []v1.Container{
|
||||
{
|
||||
Image: "test-image",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
expectedError: "name is required",
|
||||
},
|
||||
{
|
||||
name: "sidecar without image",
|
||||
spec: acidv1.PostgresSpec{
|
||||
PostgresqlParam: acidv1.PostgresqlParam{
|
||||
PgVersion: "17",
|
||||
},
|
||||
TeamID: "myapp",
|
||||
NumberOfInstances: 1,
|
||||
Volume: acidv1.Volume{
|
||||
Size: "1G",
|
||||
},
|
||||
Sidecars: []acidv1.Sidecar{
|
||||
{
|
||||
Name: "invalid-sidecar",
|
||||
},
|
||||
},
|
||||
},
|
||||
clusterConfig: Config{
|
||||
OpConfig: config.Config{
|
||||
PodManagementPolicy: "ordered_ready",
|
||||
ProtectedRoles: []string{"admin"},
|
||||
Auth: config.Auth{
|
||||
SuperUsername: superUserName,
|
||||
ReplicationUsername: replicationUserName,
|
||||
},
|
||||
},
|
||||
},
|
||||
expectedError: "image is required",
|
||||
},
|
||||
{
|
||||
name: "valid containers pass validation",
|
||||
spec: acidv1.PostgresSpec{
|
||||
PostgresqlParam: acidv1.PostgresqlParam{
|
||||
PgVersion: "17",
|
||||
},
|
||||
TeamID: "myapp",
|
||||
NumberOfInstances: 1,
|
||||
Volume: acidv1.Volume{
|
||||
Size: "1G",
|
||||
},
|
||||
Sidecars: []acidv1.Sidecar{
|
||||
{
|
||||
Name: "valid-sidecar",
|
||||
DockerImage: "busybox:latest",
|
||||
},
|
||||
},
|
||||
InitContainers: []v1.Container{
|
||||
{
|
||||
Name: "valid-initcontainer",
|
||||
Image: "alpine:latest",
|
||||
},
|
||||
},
|
||||
},
|
||||
clusterConfig: Config{
|
||||
OpConfig: config.Config{
|
||||
PodManagementPolicy: "ordered_ready",
|
||||
ProtectedRoles: []string{"admin"},
|
||||
Auth: config.Auth{
|
||||
SuperUsername: superUserName,
|
||||
ReplicationUsername: replicationUserName,
|
||||
},
|
||||
},
|
||||
},
|
||||
expectedError: "",
|
||||
},
|
||||
{
|
||||
name: "multiple invalid sidecars",
|
||||
spec: acidv1.PostgresSpec{
|
||||
Sidecars: []acidv1.Sidecar{
|
||||
{
|
||||
Name: "sidecar1",
|
||||
},
|
||||
{
|
||||
Name: "sidecar2",
|
||||
},
|
||||
},
|
||||
},
|
||||
expectedError: "image is required",
|
||||
},
|
||||
{
|
||||
name: "empty container name and image",
|
||||
spec: acidv1.PostgresSpec{
|
||||
InitContainers: []v1.Container{
|
||||
{
|
||||
Name: "",
|
||||
Image: "",
|
||||
},
|
||||
},
|
||||
},
|
||||
expectedError: "name is required",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range testCases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
cluster := New(tc.clusterConfig, k8sutil.KubernetesClient{}, acidv1.Postgresql{}, logger, eventRecorder)
|
||||
|
||||
_, err := cluster.generateStatefulSet(&tc.spec)
|
||||
|
||||
if tc.expectedError != "" {
|
||||
assert.Error(t, err)
|
||||
assert.Contains(t, err.Error(), tc.expectedError)
|
||||
} else {
|
||||
assert.NoError(t, err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestGeneratePodDisruptionBudget(t *testing.T) {
|
||||
testName := "Test PodDisruptionBudget spec generation"
|
||||
|
||||
|
|
@ -2618,7 +2798,8 @@ func TestGenerateService(t *testing.T) {
|
|||
Name: "cluster-specific-sidecar",
|
||||
},
|
||||
{
|
||||
Name: "cluster-specific-sidecar-with-resources",
|
||||
Name: "cluster-specific-sidecar-with-resources",
|
||||
DockerImage: "test-image",
|
||||
Resources: &acidv1.Resources{
|
||||
ResourceRequests: acidv1.ResourceDescription{CPU: k8sutil.StringToPointer("210m"), Memory: k8sutil.StringToPointer("0.8Gi")},
|
||||
ResourceLimits: acidv1.ResourceDescription{CPU: k8sutil.StringToPointer("510m"), Memory: k8sutil.StringToPointer("1.4Gi")},
|
||||
|
|
@ -2928,6 +3109,7 @@ func TestGenerateResourceRequirements(t *testing.T) {
|
|||
namespace := "default"
|
||||
clusterNameLabel := "cluster-name"
|
||||
sidecarName := "postgres-exporter"
|
||||
dockerImage := "test-image"
|
||||
|
||||
// enforceMinResourceLimits will be called 2 times emitting 4 events (2x cpu, 2x memory raise)
|
||||
// enforceMaxResourceRequests will be called 4 times emitting 6 events (2x cpu, 4x memory cap)
|
||||
|
|
@ -2993,7 +3175,8 @@ func TestGenerateResourceRequirements(t *testing.T) {
|
|||
Spec: acidv1.PostgresSpec{
|
||||
Sidecars: []acidv1.Sidecar{
|
||||
{
|
||||
Name: sidecarName,
|
||||
Name: sidecarName,
|
||||
DockerImage: dockerImage,
|
||||
},
|
||||
},
|
||||
TeamID: "acid",
|
||||
|
|
@ -3232,7 +3415,8 @@ func TestGenerateResourceRequirements(t *testing.T) {
|
|||
Spec: acidv1.PostgresSpec{
|
||||
Sidecars: []acidv1.Sidecar{
|
||||
{
|
||||
Name: sidecarName,
|
||||
Name: sidecarName,
|
||||
DockerImage: dockerImage,
|
||||
Resources: &acidv1.Resources{
|
||||
ResourceRequests: acidv1.ResourceDescription{CPU: k8sutil.StringToPointer("10m"), Memory: k8sutil.StringToPointer("10Mi")},
|
||||
ResourceLimits: acidv1.ResourceDescription{CPU: k8sutil.StringToPointer("100m"), Memory: k8sutil.StringToPointer("100Mi")},
|
||||
|
|
@ -3321,7 +3505,8 @@ func TestGenerateResourceRequirements(t *testing.T) {
|
|||
Spec: acidv1.PostgresSpec{
|
||||
Sidecars: []acidv1.Sidecar{
|
||||
{
|
||||
Name: sidecarName,
|
||||
Name: sidecarName,
|
||||
DockerImage: dockerImage,
|
||||
Resources: &acidv1.Resources{
|
||||
ResourceRequests: acidv1.ResourceDescription{CPU: k8sutil.StringToPointer("10m"), Memory: k8sutil.StringToPointer("10Mi")},
|
||||
ResourceLimits: acidv1.ResourceDescription{CPU: k8sutil.StringToPointer("100m"), Memory: k8sutil.StringToPointer("100Mi")},
|
||||
|
|
|
|||
Loading…
Reference in New Issue