diff --git a/api/operator/v1/vlagent_types.go b/api/operator/v1/vlagent_types.go index bac611244..9f9d87a2c 100644 --- a/api/operator/v1/vlagent_types.go +++ b/api/operator/v1/vlagent_types.go @@ -55,10 +55,10 @@ type VLAgentSpec struct { // PodDisruptionBudget created by operator // +optional PodDisruptionBudget *vmv1beta1.EmbeddedPodDisruptionBudgetSpec `json:"podDisruptionBudget,omitempty"` - // StatefulStorage configures storage for StatefulSet + // Storage configures storage for StatefulSet // +optional Storage *vmv1beta1.StorageSpec `json:"storage,omitempty"` - // StatefulRollingUpdateStrategy allows configuration for strategyType + // RollingUpdateStrategy allows configuration for strategyType // set it to RollingUpdate for disabling operator statefulSet rollingUpdate // +optional RollingUpdateStrategy appsv1.StatefulSetUpdateStrategyType `json:"rollingUpdateStrategy,omitempty"` diff --git a/config/crd/overlay/crd.yaml b/config/crd/overlay/crd.yaml index 648f4dd2d..41e7977ce 100644 --- a/config/crd/overlay/crd.yaml +++ b/config/crd/overlay/crd.yaml @@ -1255,7 +1255,7 @@ spec: type: integer rollingUpdateStrategy: description: |- - StatefulRollingUpdateStrategy allows configuration for strategyType + RollingUpdateStrategy allows configuration for strategyType set it to RollingUpdate for disabling operator statefulSet rollingUpdate type: string runtimeClassName: @@ -1345,7 +1345,7 @@ spec: type: object x-kubernetes-preserve-unknown-fields: true storage: - description: StatefulStorage configures storage for StatefulSet + description: Storage configures storage for StatefulSet properties: disableMountSubPath: description: |- diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 329b487ca..f32c72533 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -14,6 +14,7 @@ aliases: ## tip * FEATURE: [converter](https://docs.victoriametrics.com/operator/integrations/prometheus/#objects-conversion): support `spec.limit`, `spec.labels`, `spec.query_offset` and `spec.group[*].keep_firing_for` PrometheusRule properties conversion to VMRule. Related issue [#1485](https://github.com/VictoriaMetrics/operator/issues/1485). +* BUGFIX: [vmsingle](https://docs.victoriametrics.com/operator/resources/vmsingle/) and [vlsingle](https://docs.victoriametrics.com/operator/resources/vlsingle/): do not mount emptydir if storage data volume is already present in volumes list. Now it's impossible to mount external PVC without overriding default storageDataPath using `spec.extraArgs` and without having unneeded emptydir listed among pod volumes. Related issues [#1477](https://github.com/VictoriaMetrics/operator/issues/1477). * BUGFIX: [config-reloader](https://github.com/VictoriaMetrics/operator/tree/master/cmd/config-reloader): fixed config reloader command line arguments override. Related issue [#1378](https://github.com/VictoriaMetrics/operator/issues/1478). ## [v0.61.2](https://github.com/VictoriaMetrics/operator/releases/tag/v0.61.2) diff --git a/docs/api.md b/docs/api.md index d50c924f9..d10e0573e 100644 --- a/docs/api.md +++ b/docs/api.md @@ -251,7 +251,7 @@ Appears in: [VLAgent](#vlagent) | replicaCount#
_integer_ | _(Optional)_
ReplicaCount is the expected size of the Application. | | resources#
_[ResourceRequirements](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#resourcerequirements-v1-core)_ | _(Optional)_
Resources container resource request and limits, https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
if not defined default resources from operator config will be used | | revisionHistoryLimitCount#
_integer_ | _(Optional)_
The number of old ReplicaSets to retain to allow rollback in deployment or
maximum number of revisions that will be maintained in the Deployment revision history.
Has no effect at StatefulSets
Defaults to 10. | -| rollingUpdateStrategy#
_[StatefulSetUpdateStrategyType](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#statefulsetupdatestrategytype-v1-apps)_ | _(Optional)_
StatefulRollingUpdateStrategy allows configuration for strategyType
set it to RollingUpdate for disabling operator statefulSet rollingUpdate | +| rollingUpdateStrategy#
_[StatefulSetUpdateStrategyType](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#statefulsetupdatestrategytype-v1-apps)_ | _(Optional)_
RollingUpdateStrategy allows configuration for strategyType
set it to RollingUpdate for disabling operator statefulSet rollingUpdate | | runtimeClassName#
_string_ | _(Optional)_
RuntimeClassName - defines runtime class for kubernetes pod.
https://kubernetes.io/docs/concepts/containers/runtime-class/ | | schedulerName#
_string_ | _(Optional)_
SchedulerName - defines kubernetes scheduler name | | secrets#
_string array_ | _(Optional)_
Secrets is a list of Secrets in the same namespace as the Application
object, which shall be mounted into the Application container
at /etc/vm/secrets/SECRET_NAME folder | @@ -259,7 +259,7 @@ Appears in: [VLAgent](#vlagent) | serviceAccountName#
_string_ | _(Optional)_
ServiceAccountName is the name of the ServiceAccount to use to run the pods | | serviceScrapeSpec#
_[VMServiceScrapeSpec](#vmservicescrapespec)_ | _(Optional)_
ServiceScrapeSpec that will be added to vlagent VMServiceScrape spec | | serviceSpec#
_[AdditionalServiceSpec](#additionalservicespec)_ | _(Optional)_
ServiceSpec that will be added to vlagent service spec | -| storage#
_[StorageSpec](#storagespec)_ | _(Optional)_
StatefulStorage configures storage for StatefulSet | +| storage#
_[StorageSpec](#storagespec)_ | _(Optional)_
Storage configures storage for StatefulSet | | syslogSpec#
_[SyslogServerSpec](#syslogserverspec)_ | _(Optional)_
SyslogSpec defines syslog listener configuration | | terminationGracePeriodSeconds#
_integer_ | _(Optional)_
TerminationGracePeriodSeconds period for container graceful termination | | tolerations#
_[Toleration](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#toleration-v1-core) array_ | _(Optional)_
Tolerations If specified, the pod's tolerations. | diff --git a/internal/controller/operator/factory/build/container.go b/internal/controller/operator/factory/build/container.go index 66f289b0f..a304ee3c3 100644 --- a/internal/controller/operator/factory/build/container.go +++ b/internal/controller/operator/factory/build/container.go @@ -535,3 +535,47 @@ func AddSyslogTLSConfigToVolumes(dstVolumes []corev1.Volume, dstMounts []corev1. } return dstVolumes, dstMounts } + +func StorageVolumeMountsTo(volumes []corev1.Volume, mounts []corev1.VolumeMount, pvcSrc *corev1.PersistentVolumeClaimVolumeSource, volumeName, storagePath string) ([]corev1.Volume, []corev1.VolumeMount) { + var alreadyMounted bool + for _, volumeMount := range mounts { + if volumeMount.Name == volumeName { + alreadyMounted = true + break + } + } + if !alreadyMounted { + mounts = append(mounts, corev1.VolumeMount{ + Name: volumeName, + MountPath: storagePath, + }) + } + if pvcSrc != nil { + volumes = append(volumes, corev1.Volume{ + Name: volumeName, + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: pvcSrc, + }, + }) + return volumes, mounts + } + + var volumePresent bool + for _, volume := range volumes { + if volume.Name == volumeName { + volumePresent = true + break + } + } + if volumePresent { + return volumes, mounts + } + + volumes = append(volumes, corev1.Volume{ + Name: volumeName, + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }) + return volumes, mounts +} diff --git a/internal/controller/operator/factory/build/container_test.go b/internal/controller/operator/factory/build/container_test.go index dc29ab22c..ffc53c108 100644 --- a/internal/controller/operator/factory/build/container_test.go +++ b/internal/controller/operator/factory/build/container_test.go @@ -41,220 +41,206 @@ func (t testBuildProbeCR) ProbeNeedLiveness() bool { } func Test_buildProbe(t *testing.T) { - type args struct { + type opts struct { container corev1.Container cr testBuildProbeCR + validate func(corev1.Container) error } - tests := []struct { - name string - args args - validate func(corev1.Container) error - }{ - { - name: "build default probe with empty ep", - args: args{ - cr: testBuildProbeCR{ - probePath: func() string { - return "/health" - }, - port: "8051", - needAddLiveness: true, - scheme: "HTTP", - }, - container: corev1.Container{}, - }, - validate: func(container corev1.Container) error { - if container.LivenessProbe == nil { - return fmt.Errorf("want liveness to be not nil") - } - if container.ReadinessProbe == nil { - return fmt.Errorf("want readinessProbe to be not nil") - } - if container.ReadinessProbe.HTTPGet.Scheme != "HTTP" { - return fmt.Errorf("expect scheme to be HTTP got: %s", container.ReadinessProbe.HTTPGet.Scheme) - } - return nil + f := func(opts opts) { + t.Helper() + got := Probe(opts.container, opts.cr) + if err := opts.validate(got); err != nil { + t.Errorf("buildProbe() unexpected error: %v", err) + } + } + + // build default probe with empty ep + o := opts{ + cr: testBuildProbeCR{ + probePath: func() string { + return "/health" }, + port: "8051", + needAddLiveness: true, + scheme: "HTTP", }, - { - name: "build default probe with empty ep using HTTPS", - args: args{ - cr: testBuildProbeCR{ - probePath: func() string { - return "/health" - }, - port: "8051", - needAddLiveness: true, - scheme: "HTTPS", - }, - container: corev1.Container{}, - }, - validate: func(container corev1.Container) error { - if container.LivenessProbe == nil { - return fmt.Errorf("want liveness to be not nil") - } - if container.ReadinessProbe == nil { - return fmt.Errorf("want readinessProbe to be not nil") - } - if container.LivenessProbe.HTTPGet.Scheme != "HTTPS" { - return fmt.Errorf("expect scheme to be HTTPS got: %s", container.LivenessProbe.HTTPGet.Scheme) - } - if container.ReadinessProbe.HTTPGet.Scheme != "HTTPS" { - return fmt.Errorf("expect scheme to be HTTPS got: %s", container.ReadinessProbe.HTTPGet.Scheme) - } - return nil + container: corev1.Container{}, + validate: func(container corev1.Container) error { + if container.LivenessProbe == nil { + return fmt.Errorf("want liveness to be not nil") + } + if container.ReadinessProbe == nil { + return fmt.Errorf("want readinessProbe to be not nil") + } + if container.ReadinessProbe.HTTPGet.Scheme != "HTTP" { + return fmt.Errorf("expect scheme to be HTTP got: %s", container.ReadinessProbe.HTTPGet.Scheme) + } + return nil + }, + } + f(o) + + // build default probe with empty ep using HTTPS + o = opts{ + cr: testBuildProbeCR{ + probePath: func() string { + return "/health" }, + port: "8051", + needAddLiveness: true, + scheme: "HTTPS", }, - { - name: "build default probe with ep", - args: args{ - cr: testBuildProbeCR{ - probePath: func() string { - return "/health" - }, - port: "8051", - needAddLiveness: true, - ep: &vmv1beta1.EmbeddedProbes{ - ReadinessProbe: &corev1.Probe{ - ProbeHandler: corev1.ProbeHandler{ - Exec: &corev1.ExecAction{ - Command: []string{"echo", "1"}, - }, - }, + container: corev1.Container{}, + validate: func(container corev1.Container) error { + if container.LivenessProbe == nil { + return fmt.Errorf("want liveness to be not nil") + } + if container.ReadinessProbe == nil { + return fmt.Errorf("want readinessProbe to be not nil") + } + if container.LivenessProbe.HTTPGet.Scheme != "HTTPS" { + return fmt.Errorf("expect scheme to be HTTPS got: %s", container.LivenessProbe.HTTPGet.Scheme) + } + if container.ReadinessProbe.HTTPGet.Scheme != "HTTPS" { + return fmt.Errorf("expect scheme to be HTTPS got: %s", container.ReadinessProbe.HTTPGet.Scheme) + } + return nil + }, + } + f(o) + + // build default probe with ep + o = opts{ + cr: testBuildProbeCR{ + probePath: func() string { + return "/health" + }, + port: "8051", + needAddLiveness: true, + ep: &vmv1beta1.EmbeddedProbes{ + ReadinessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + Exec: &corev1.ExecAction{ + Command: []string{"echo", "1"}, }, - StartupProbe: &corev1.Probe{ - ProbeHandler: corev1.ProbeHandler{ - HTTPGet: &corev1.HTTPGetAction{ - Host: "some", - }, - }, + }, + }, + StartupProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Host: "some", }, - LivenessProbe: &corev1.Probe{ - ProbeHandler: corev1.ProbeHandler{ - HTTPGet: &corev1.HTTPGetAction{ - Path: "/live1", - }, - }, - TimeoutSeconds: 15, - InitialDelaySeconds: 20, + }, + }, + LivenessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/live1", }, }, + TimeoutSeconds: 15, + InitialDelaySeconds: 20, }, - container: corev1.Container{}, - }, - validate: func(container corev1.Container) error { - if container.LivenessProbe == nil { - return fmt.Errorf("want liveness to be not nil") - } - if container.ReadinessProbe == nil { - return fmt.Errorf("want readinessProbe to be not nil") - } - if container.StartupProbe == nil { - return fmt.Errorf("want startupProbe to be not nil") - } - if len(container.ReadinessProbe.Exec.Command) != 2 { - return fmt.Errorf("want exec args: %d, got: %v", 2, container.ReadinessProbe.Exec.Command) - } - if container.StartupProbe.HTTPGet.Host != "some" { - return fmt.Errorf("want host: %s, got: %s", "some", container.StartupProbe.HTTPGet.Host) - } - if container.LivenessProbe.HTTPGet.Path != "/live1" { - return fmt.Errorf("unexpected path, got: %s, want: %v", container.LivenessProbe.HTTPGet.Path, "/live1") - } - if container.LivenessProbe.InitialDelaySeconds != 20 { - return fmt.Errorf("unexpected delay, got: %d, want: %d", container.LivenessProbe.InitialDelaySeconds, 20) - } - if container.LivenessProbe.TimeoutSeconds != 15 { - return fmt.Errorf("unexpected timeout, got: %d, want: %d", container.LivenessProbe.TimeoutSeconds, 15) - } - return nil }, }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := Probe(tt.args.container, tt.args.cr) - if err := tt.validate(got); err != nil { - t.Errorf("buildProbe() unexpected error: %v", err) + container: corev1.Container{}, + validate: func(container corev1.Container) error { + if container.LivenessProbe == nil { + return fmt.Errorf("want liveness to be not nil") + } + if container.ReadinessProbe == nil { + return fmt.Errorf("want readinessProbe to be not nil") + } + if container.StartupProbe == nil { + return fmt.Errorf("want startupProbe to be not nil") + } + if len(container.ReadinessProbe.Exec.Command) != 2 { + return fmt.Errorf("want exec args: %d, got: %v", 2, container.ReadinessProbe.Exec.Command) } - }) + if container.StartupProbe.HTTPGet.Host != "some" { + return fmt.Errorf("want host: %s, got: %s", "some", container.StartupProbe.HTTPGet.Host) + } + if container.LivenessProbe.HTTPGet.Path != "/live1" { + return fmt.Errorf("unexpected path, got: %s, want: %v", container.LivenessProbe.HTTPGet.Path, "/live1") + } + if container.LivenessProbe.InitialDelaySeconds != 20 { + return fmt.Errorf("unexpected delay, got: %d, want: %d", container.LivenessProbe.InitialDelaySeconds, 20) + } + if container.LivenessProbe.TimeoutSeconds != 15 { + return fmt.Errorf("unexpected timeout, got: %d, want: %d", container.LivenessProbe.TimeoutSeconds, 15) + } + return nil + }, } + f(o) } func Test_addExtraArgsOverrideDefaults(t *testing.T) { - type args struct { + type opts struct { args []string extraArgs map[string]string dashes string + want []string } - tests := []struct { - name string - args args - want []string - }{ - { - name: "no changes", - args: args{ - args: []string{"-http.ListenAddr=:8081"}, - dashes: "-", - }, - want: []string{"-http.ListenAddr=:8081"}, - }, - { - name: "override default", - args: args{ - args: []string{"-http.ListenAddr=:8081"}, - extraArgs: map[string]string{"http.ListenAddr": "127.0.0.1:8085"}, - dashes: "-", - }, - want: []string{"-http.ListenAddr=127.0.0.1:8085"}, - }, - { - name: "override default, add to the end", - args: args{ - args: []string{"-http.ListenAddr=:8081", "-promscrape.config=/opt/vmagent.yml"}, - extraArgs: map[string]string{"http.ListenAddr": "127.0.0.1:8085"}, - dashes: "-", - }, - want: []string{"-promscrape.config=/opt/vmagent.yml", "-http.ListenAddr=127.0.0.1:8085"}, - }, - { - name: "two dashes, extend", - args: args{ - args: []string{"--web.timeout=0"}, - extraArgs: map[string]string{"log.level": "debug"}, - dashes: "--", - }, - want: []string{"--web.timeout=0", "--log.level=debug"}, - }, - { - name: "two dashes, override default", - args: args{ - args: []string{"--log.level=info"}, - extraArgs: map[string]string{"log.level": "debug"}, - dashes: "--", - }, - want: []string{"--log.level=debug"}, - }, - { - name: "two dashes, alertmanager migration", - args: args{ - args: []string{"--log.level=info"}, - extraArgs: map[string]string{"-web.externalURL": "http://domain.example"}, - dashes: "--", - }, - want: []string{"--log.level=info", "--web.externalURL=http://domain.example"}, - }, + f := func(opts opts) { + t.Helper() + assert.Equalf(t, opts.want, + AddExtraArgsOverrideDefaults(opts.args, opts.extraArgs, opts.dashes), + "addExtraArgsOverrideDefaults(%v, %v)", opts.args, opts.extraArgs) } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - assert.Equalf( - t, - tt.want, - AddExtraArgsOverrideDefaults(tt.args.args, tt.args.extraArgs, tt.args.dashes), - "addExtraArgsOverrideDefaults(%v, %v)", tt.args.args, tt.args.extraArgs) - }) + + // no changes + o := opts{ + args: []string{"-http.ListenAddr=:8081"}, + dashes: "-", + want: []string{"-http.ListenAddr=:8081"}, + } + f(o) + + // override default + o = opts{ + args: []string{"-http.ListenAddr=:8081"}, + extraArgs: map[string]string{"http.ListenAddr": "127.0.0.1:8085"}, + dashes: "-", + want: []string{"-http.ListenAddr=127.0.0.1:8085"}, + } + f(o) + + // override default, add to the end + o = opts{ + args: []string{"-http.ListenAddr=:8081", "-promscrape.config=/opt/vmagent.yml"}, + extraArgs: map[string]string{"http.ListenAddr": "127.0.0.1:8085"}, + dashes: "-", + want: []string{"-promscrape.config=/opt/vmagent.yml", "-http.ListenAddr=127.0.0.1:8085"}, + } + f(o) + + // two dashes, extend + o = opts{ + args: []string{"--web.timeout=0"}, + extraArgs: map[string]string{"log.level": "debug"}, + dashes: "--", + want: []string{"--web.timeout=0", "--log.level=debug"}, + } + f(o) + + // two dashes, override default + o = opts{ + args: []string{"--log.level=info"}, + extraArgs: map[string]string{"log.level": "debug"}, + dashes: "--", + want: []string{"--log.level=debug"}, } + f(o) + + // two dashes, alertmanager migration + o = opts{ + args: []string{"--log.level=info"}, + extraArgs: map[string]string{"-web.externalURL": "http://domain.example"}, + dashes: "--", + want: []string{"--log.level=info", "--web.externalURL=http://domain.example"}, + } + f(o) } func TestFormatContainerImage(t *testing.T) { @@ -279,128 +265,324 @@ func TestFormatContainerImage(t *testing.T) { } func TestAddSyslogArgsTo(t *testing.T) { - f := func(syslogSpec *vmv1.SyslogServerSpec, wantArgs []string) { + type opts struct { + spec *vmv1.SyslogServerSpec + expected []string + } + f := func(opts opts) { t.Helper() - args := AddSyslogArgsTo(nil, syslogSpec, "/etc/vm/tls-server-secrets") + args := AddSyslogArgsTo(nil, opts.spec, "/etc/vm/tls-server-secrets") sort.Strings(args) - sort.Strings(wantArgs) - assert.Equal(t, wantArgs, args) + sort.Strings(opts.expected) + assert.Equal(t, opts.expected, args) } - f(nil, nil) + + // empty + o := opts{} + f(o) + // multiple tcp listeners - spec := vmv1.SyslogServerSpec{ - TCPListeners: []*vmv1.SyslogTCPListener{ - { - ListenPort: 3001, - }, - { - ListenPort: 3002, - StreamFields: vmv1.FieldsListString(`["msg_1","msg_2"]`), + o = opts{ + spec: &vmv1.SyslogServerSpec{ + TCPListeners: []*vmv1.SyslogTCPListener{ + { + ListenPort: 3001, + }, + { + ListenPort: 3002, + StreamFields: vmv1.FieldsListString(`["msg_1","msg_2"]`), + }, }, }, + expected: []string{ + "-syslog.listenAddr.tcp=:3001,:3002", + `-syslog.streamFields.tcp='','["msg_1","msg_2"]'`, + }, } - expected := []string{ - "-syslog.listenAddr.tcp=:3001,:3002", - `-syslog.streamFields.tcp='','["msg_1","msg_2"]'`, + f(o) + + // multiple udp listeners + o = opts{ + spec: &vmv1.SyslogServerSpec{ + UDPListeners: []*vmv1.SyslogUDPListener{ + { + ListenPort: 3001, + IgnoreFields: vmv1.FieldsListString(`["ignore_1"]`), + }, + { + ListenPort: 3002, + StreamFields: vmv1.FieldsListString(`["msg_1","msg_2"]`), + }, + { + ListenPort: 3005, + }, + }, + }, + expected: []string{ + "-syslog.listenAddr.udp=:3001,:3002,:3005", + `-syslog.streamFields.udp='','["msg_1","msg_2"]',''`, + `-syslog.ignoreFields.udp='["ignore_1"]','',''`, + }, } - f(&spec, expected) + f(o) + // mixed udp and tcp // multiple udp listeners - spec = vmv1.SyslogServerSpec{ - UDPListeners: []*vmv1.SyslogUDPListener{ - { - ListenPort: 3001, - IgnoreFields: vmv1.FieldsListString(`["ignore_1"]`), + o = opts{ + spec: &vmv1.SyslogServerSpec{ + TCPListeners: []*vmv1.SyslogTCPListener{ + { + ListenPort: 3001, + }, + { + ListenPort: 3002, + StreamFields: vmv1.FieldsListString(`["msg_1","msg_2"]`), + }, }, - { - ListenPort: 3002, - StreamFields: vmv1.FieldsListString(`["msg_1","msg_2"]`), + UDPListeners: []*vmv1.SyslogUDPListener{ + { + ListenPort: 3001, + IgnoreFields: vmv1.FieldsListString(`["ignore_1"]`), + }, + { + ListenPort: 3002, + StreamFields: vmv1.FieldsListString(`["msg_1","msg_2"]`), + }, + { + ListenPort: 3005, + }, }, - { - ListenPort: 3005, + }, + expected: []string{ + "-syslog.listenAddr.tcp=:3001,:3002", + `-syslog.streamFields.tcp='','["msg_1","msg_2"]'`, + "-syslog.listenAddr.udp=:3001,:3002,:3005", + `-syslog.streamFields.udp='','["msg_1","msg_2"]',''`, + `-syslog.ignoreFields.udp='["ignore_1"]','',''`, + }, + } + f(o) + + // with tls + o = opts{ + spec: &vmv1.SyslogServerSpec{ + TCPListeners: []*vmv1.SyslogTCPListener{ + { + ListenPort: 3001, + TenantID: "10:25", + }, + { + ListenPort: 3002, + StreamFields: vmv1.FieldsListString(`["msg_1","msg_2"]`), + TLSConfig: &vmv1.TLSServerConfig{ + CertSecret: &corev1.SecretKeySelector{ + Key: "CERT", + LocalObjectReference: corev1.LocalObjectReference{ + Name: "tls", + }, + }, + KeyFile: "/etc/vm/secrets/tls/key", + }, + }, + }, + UDPListeners: []*vmv1.SyslogUDPListener{ + { + ListenPort: 3001, + CompressMethod: "zstd", + }, }, }, + expected: []string{ + "-syslog.listenAddr.tcp=:3001,:3002", + `-syslog.streamFields.tcp='','["msg_1","msg_2"]'`, + `-syslog.tenantID.tcp=10:25,`, + "-syslog.tls=,true", + "-syslog.tlsCertFile=,/etc/vm/tls-server-secrets/tls/CERT", + "-syslog.tlsKeyFile=,/etc/vm/secrets/tls/key", + "-syslog.listenAddr.udp=:3001", + "-syslog.compressMethod.udp=zstd", + }, } - expected = []string{ - "-syslog.listenAddr.udp=:3001,:3002,:3005", - `-syslog.streamFields.udp='','["msg_1","msg_2"]',''`, - `-syslog.ignoreFields.udp='["ignore_1"]','',''`, + f(o) +} + +func TestStorageVolumeMountsTo(t *testing.T) { + type opts struct { + pvcSrc *corev1.PersistentVolumeClaimVolumeSource + volumeName string + storagePath string + volumes []corev1.Volume + expectedVolumes []corev1.Volume + mounts []corev1.VolumeMount + expectedMounts []corev1.VolumeMount + } + f := func(opts opts) { + t.Helper() + gotVolumes, gotMounts := StorageVolumeMountsTo(opts.volumes, opts.mounts, opts.pvcSrc, opts.volumeName, opts.storagePath) + assert.Equal(t, opts.expectedMounts, gotMounts) + assert.Equal(t, opts.expectedVolumes, gotVolumes) } - f(&spec, expected) - // mixed udp and tcp - // multiple udp listeners - spec = vmv1.SyslogServerSpec{ - TCPListeners: []*vmv1.SyslogTCPListener{ + // no PVC spec and no volumes and mounts + o := opts{ + volumeName: "test", + storagePath: "/test", + expectedVolumes: []corev1.Volume{{ + Name: "test", + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }}, + expectedMounts: []corev1.VolumeMount{{ + Name: "test", + MountPath: "/test", + }}, + } + f(o) + + // with PVC spec and no volumes and mounts + o = opts{ + volumeName: "test", + storagePath: "/test", + pvcSrc: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "test-claim", + }, + expectedVolumes: []corev1.Volume{{ + Name: "test", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "test-claim", + }, + }, + }}, + expectedMounts: []corev1.VolumeMount{{ + Name: "test", + MountPath: "/test", + }}, + } + f(o) + + // with PVC spec and matching data volume + o = opts{ + volumes: []corev1.Volume{{ + Name: "test", + VolumeSource: corev1.VolumeSource{ + AWSElasticBlockStore: &corev1.AWSElasticBlockStoreVolumeSource{ + VolumeID: "aws-volume", + }, + }, + }}, + volumeName: "test", + storagePath: "/test", + pvcSrc: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "test-claim", + }, + expectedVolumes: []corev1.Volume{ { - ListenPort: 3001, + Name: "test", + VolumeSource: corev1.VolumeSource{ + AWSElasticBlockStore: &corev1.AWSElasticBlockStoreVolumeSource{ + VolumeID: "aws-volume", + }, + }, }, { - ListenPort: 3002, - StreamFields: vmv1.FieldsListString(`["msg_1","msg_2"]`), + Name: "test", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "test-claim", + }, + }, }, }, - UDPListeners: []*vmv1.SyslogUDPListener{ - { - ListenPort: 3001, - IgnoreFields: vmv1.FieldsListString(`["ignore_1"]`), + expectedMounts: []corev1.VolumeMount{{ + Name: "test", + MountPath: "/test", + }}, + } + f(o) + + // with PVC spec and not matching data volume + o = opts{ + volumes: []corev1.Volume{{ + Name: "extra", + VolumeSource: corev1.VolumeSource{ + AWSElasticBlockStore: &corev1.AWSElasticBlockStoreVolumeSource{ + VolumeID: "aws-volume", + }, }, + }}, + volumeName: "test", + storagePath: "/test", + pvcSrc: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "test-claim", + }, + expectedVolumes: []corev1.Volume{ { - ListenPort: 3002, - StreamFields: vmv1.FieldsListString(`["msg_1","msg_2"]`), + Name: "extra", + VolumeSource: corev1.VolumeSource{ + AWSElasticBlockStore: &corev1.AWSElasticBlockStoreVolumeSource{ + VolumeID: "aws-volume", + }, + }, }, { - ListenPort: 3005, + Name: "test", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "test-claim", + }, + }, }, }, + expectedMounts: []corev1.VolumeMount{{ + Name: "test", + MountPath: "/test", + }}, } - expected = []string{ - "-syslog.listenAddr.tcp=:3001,:3002", - `-syslog.streamFields.tcp='','["msg_1","msg_2"]'`, - "-syslog.listenAddr.udp=:3001,:3002,:3005", - `-syslog.streamFields.udp='','["msg_1","msg_2"]',''`, - `-syslog.ignoreFields.udp='["ignore_1"]','',''`, - } - f(&spec, expected) + f(o) - // with tls - spec = vmv1.SyslogServerSpec{ - TCPListeners: []*vmv1.SyslogTCPListener{ - { - ListenPort: 3001, - TenantID: "10:25", + // with PVC spec and existing data volume mount + o = opts{ + volumes: []corev1.Volume{{ + Name: "extra", + VolumeSource: corev1.VolumeSource{ + AWSElasticBlockStore: &corev1.AWSElasticBlockStoreVolumeSource{ + VolumeID: "aws-volume", + }, }, + }}, + mounts: []corev1.VolumeMount{{ + Name: "test", + MountPath: "/other-path", + }}, + volumeName: "test", + storagePath: "/test", + pvcSrc: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "test-claim", + }, + expectedVolumes: []corev1.Volume{ { - ListenPort: 3002, - StreamFields: vmv1.FieldsListString(`["msg_1","msg_2"]`), - TLSConfig: &vmv1.TLSServerConfig{ - CertSecret: &corev1.SecretKeySelector{ - Key: "CERT", - LocalObjectReference: corev1.LocalObjectReference{ - Name: "tls", - }, + Name: "extra", + VolumeSource: corev1.VolumeSource{ + AWSElasticBlockStore: &corev1.AWSElasticBlockStoreVolumeSource{ + VolumeID: "aws-volume", }, - KeyFile: "/etc/vm/secrets/tls/key", }, }, - }, - UDPListeners: []*vmv1.SyslogUDPListener{ { - ListenPort: 3001, - CompressMethod: "zstd", + Name: "test", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "test-claim", + }, + }, }, }, + expectedMounts: []corev1.VolumeMount{{ + Name: "test", + MountPath: "/other-path", + }}, } - expected = []string{ - "-syslog.listenAddr.tcp=:3001,:3002", - `-syslog.streamFields.tcp='','["msg_1","msg_2"]'`, - `-syslog.tenantID.tcp=10:25,`, - "-syslog.tls=,true", - "-syslog.tlsCertFile=,/etc/vm/tls-server-secrets/tls/CERT", - "-syslog.tlsKeyFile=,/etc/vm/secrets/tls/key", - "-syslog.listenAddr.udp=:3001", - "-syslog.compressMethod.udp=zstd", - } - f(&spec, expected) - + f(o) } diff --git a/internal/controller/operator/factory/vlsingle/vlsingle.go b/internal/controller/operator/factory/vlsingle/vlsingle.go index 12aeeb237..4da0ab669 100644 --- a/internal/controller/operator/factory/vlsingle/vlsingle.go +++ b/internal/controller/operator/factory/vlsingle/vlsingle.go @@ -68,7 +68,7 @@ func CreateOrUpdate(ctx context.Context, rclient client.Client, cr *vmv1.VLSingl if err := deletePrevStateResources(ctx, cr, rclient); err != nil { return err } - if cr.Spec.Storage != nil && cr.Spec.StorageDataPath == "" { + if cr.Spec.Storage != nil { if err := createOrUpdatePVC(ctx, rclient, cr, prevCR); err != nil { return err } @@ -153,9 +153,6 @@ func makePodSpec(r *vmv1.VLSingle) (*corev1.PodTemplateSpec, error) { args = append(args, fmt.Sprintf("-retention.maxDiskSpaceUsageBytes=%s", r.Spec.RetentionMaxDiskSpaceUsageBytes)) } - // if customStorageDataPath is not empty, do not add pvc. - shouldAddPVC := r.Spec.StorageDataPath == "" - storagePath := vlsingleDataDir if r.Spec.StorageDataPath != "" { storagePath = r.Spec.StorageDataPath @@ -186,37 +183,19 @@ func makePodSpec(r *vmv1.VLSingle) (*corev1.PodTemplateSpec, error) { var ports []corev1.ContainerPort ports = append(ports, corev1.ContainerPort{Name: "http", Protocol: "TCP", ContainerPort: intstr.Parse(r.Spec.Port).IntVal}) - volumes := []corev1.Volume{} - - storageSpec := r.Spec.Storage - - if storageSpec == nil { - volumes = append(volumes, corev1.Volume{ - Name: vlsingleDataVolumeName, - VolumeSource: corev1.VolumeSource{ - EmptyDir: &corev1.EmptyDirVolumeSource{}, - }, - }) - } else if shouldAddPVC { - volumes = append(volumes, corev1.Volume{ - Name: vlsingleDataVolumeName, - VolumeSource: corev1.VolumeSource{ - PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ - ClaimName: r.PrefixedName(), - }, - }, - }) - } + var volumes []corev1.Volume + var vmMounts []corev1.VolumeMount volumes = append(volumes, r.Spec.Volumes...) - vmMounts := []corev1.VolumeMount{ - { - Name: vlsingleDataVolumeName, - MountPath: storagePath, - }, - } - vmMounts = append(vmMounts, r.Spec.VolumeMounts...) + var pvcSrc *corev1.PersistentVolumeClaimVolumeSource + if r.Spec.Storage != nil { + pvcSrc = &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: r.PrefixedName(), + } + } + volumes, vmMounts = build.StorageVolumeMountsTo(volumes, vmMounts, pvcSrc, vlsingleDataVolumeName, storagePath) + for _, s := range r.Spec.Secrets { volumes = append(volumes, corev1.Volume{ Name: k8stools.SanitizeVolumeName("secret-" + s), diff --git a/internal/controller/operator/factory/vmsingle/vmsingle.go b/internal/controller/operator/factory/vmsingle/vmsingle.go index 50453b30f..60ebe8d67 100644 --- a/internal/controller/operator/factory/vmsingle/vmsingle.go +++ b/internal/controller/operator/factory/vmsingle/vmsingle.go @@ -82,7 +82,7 @@ func CreateOrUpdate(ctx context.Context, cr *vmv1beta1.VMSingle, rclient client. } } - if cr.Spec.Storage != nil && cr.Spec.StorageDataPath == "" { + if cr.Spec.Storage != nil { if err := createStorage(ctx, rclient, cr, prevCR); err != nil { return fmt.Errorf("cannot create storage: %w", err) } @@ -151,11 +151,6 @@ func makeSpec(ctx context.Context, cr *vmv1beta1.VMSingle) (*corev1.PodTemplateS args = append(args, fmt.Sprintf("-retentionPeriod=%s", cr.Spec.RetentionPeriod)) } - // if customStorageDataPath is not empty, do not add volumes - // and volumeMounts - // it's user responsibility to provide correct values - mustAddVolumeMounts := cr.Spec.StorageDataPath == "" - storagePath := vmSingleDataDir if cr.Spec.StorageDataPath != "" { storagePath = cr.Spec.StorageDataPath @@ -184,8 +179,17 @@ func makeSpec(ctx context.Context, cr *vmv1beta1.VMSingle) (*corev1.PodTemplateS var volumes []corev1.Volume var vmMounts []corev1.VolumeMount - volumes, vmMounts = addVolumeMountsTo(volumes, vmMounts, cr, mustAddVolumeMounts, storagePath) + volumes = append(volumes, cr.Spec.Volumes...) + vmMounts = append(vmMounts, cr.Spec.VolumeMounts...) + var pvcSpec *corev1.PersistentVolumeClaimVolumeSource + if cr.Spec.Storage != nil { + pvcSpec = &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: cr.PrefixedName(), + } + } + + volumes, vmMounts = build.StorageVolumeMountsTo(volumes, vmMounts, pvcSpec, vmDataVolumeName, storagePath) if cr.Spec.VMBackup != nil && cr.Spec.VMBackup.CredentialsSecret != nil { volumes = append(volumes, corev1.Volume{ Name: k8stools.SanitizeVolumeName("secret-" + cr.Spec.VMBackup.CredentialsSecret.Name), @@ -197,9 +201,6 @@ func makeSpec(ctx context.Context, cr *vmv1beta1.VMSingle) (*corev1.PodTemplateS }) } - volumes = append(volumes, cr.Spec.Volumes...) - vmMounts = append(vmMounts, cr.Spec.VolumeMounts...) - for _, s := range cr.Spec.Secrets { volumes = append(volumes, corev1.Volume{ Name: k8stools.SanitizeVolumeName("secret-" + s), @@ -437,58 +438,3 @@ func deletePrevStateResources(ctx context.Context, rclient client.Client, cr, pr return nil } - -func addVolumeMountsTo(volumes []corev1.Volume, vmMounts []corev1.VolumeMount, cr *vmv1beta1.VMSingle, mustAddVolumeMounts bool, storagePath string) ([]corev1.Volume, []corev1.VolumeMount) { - - switch { - case mustAddVolumeMounts: - // add volume and mount point by operator directly - vmMounts = append(vmMounts, corev1.VolumeMount{ - Name: vmDataVolumeName, - MountPath: storagePath}, - ) - - vlSource := corev1.VolumeSource{ - EmptyDir: &corev1.EmptyDirVolumeSource{}, - } - if cr.Spec.Storage != nil { - vlSource = corev1.VolumeSource{ - PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ - ClaimName: cr.PrefixedName(), - }, - } - } - volumes = append(volumes, corev1.Volume{ - Name: vmDataVolumeName, - VolumeSource: vlSource}) - - case len(cr.Spec.Volumes) > 0: - // add missing volumeMount point for backward compatibility - // it simplifies management of external PVCs - var volumeNamePresent bool - for _, volume := range cr.Spec.Volumes { - if volume.Name == vmDataVolumeName { - volumeNamePresent = true - break - } - } - if volumeNamePresent { - var mustSkipVolumeAdd bool - for _, volumeMount := range cr.Spec.VolumeMounts { - if volumeMount.Name == vmDataVolumeName { - mustSkipVolumeAdd = true - break - } - } - if !mustSkipVolumeAdd { - vmMounts = append(vmMounts, corev1.VolumeMount{ - Name: vmDataVolumeName, - MountPath: storagePath, - }) - } - } - - } - - return volumes, vmMounts -} diff --git a/test/e2e/vlsingle_test.go b/test/e2e/vlsingle_test.go index c3eda1379..865ccff0d 100644 --- a/test/e2e/vlsingle_test.go +++ b/test/e2e/vlsingle_test.go @@ -148,7 +148,6 @@ var _ = Describe("test vlsingle Controller", Label("vl", "single", "vlsingle"), }, RetentionPeriod: "1", StorageDataPath: "/custom-path/internal/dir", - Storage: &corev1.PersistentVolumeClaimSpec{}, }, }, func(cr *vmv1.VLSingle) { @@ -159,8 +158,8 @@ var _ = Describe("test vlsingle Controller", Label("vl", "single", "vlsingle"), Expect(ts.Containers).To(HaveLen(1)) Expect(ts.Volumes).To(HaveLen(2)) Expect(ts.Containers[0].VolumeMounts).To(HaveLen(2)) - Expect(ts.Containers[0].VolumeMounts[0].Name).To(Equal("data")) - Expect(ts.Containers[0].VolumeMounts[1].Name).To(Equal("unused")) + Expect(ts.Containers[0].VolumeMounts[0].Name).To(Equal("unused")) + Expect(ts.Containers[0].VolumeMounts[1].Name).To(Equal("data")) }), ) diff --git a/test/e2e/vmsingle_test.go b/test/e2e/vmsingle_test.go index 5bc7d925d..c8a5ce716 100644 --- a/test/e2e/vmsingle_test.go +++ b/test/e2e/vmsingle_test.go @@ -218,7 +218,7 @@ var _ = Describe("test vmsingle Controller", Label("vm", "single"), func() { Expect(*createdDeploy.Spec.Template.Spec.Containers[0].SecurityContext.RunAsNonRoot).To(BeTrue()) }), - Entry("with data emptyDir", "emptydir", false, + Entry("with storage", "storage", false, &vmv1beta1.VMSingle{ ObjectMeta: metav1.ObjectMeta{ Namespace: namespace, @@ -248,8 +248,34 @@ var _ = Describe("test vmsingle Controller", Label("vm", "single"), func() { Expect(k8sClient.Get(ctx, createdChildObjects, &createdDeploy)).To(Succeed()) ts := createdDeploy.Spec.Template.Spec Expect(ts.Containers).To(HaveLen(1)) - Expect(ts.Volumes).To(BeEmpty()) - Expect(ts.Containers[0].VolumeMounts).To(BeEmpty()) + Expect(ts.Volumes).To(HaveLen(1)) + Expect(ts.Containers[0].VolumeMounts).To(HaveLen(1)) + }), + Entry("with empty dir", "emptydir", false, + &vmv1beta1.VMSingle{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + }, + Spec: vmv1beta1.VMSingleSpec{ + CommonApplicationDeploymentParams: vmv1beta1.CommonApplicationDeploymentParams{ + ReplicaCount: ptr.To[int32](1), + }, + CommonDefaultableParams: vmv1beta1.CommonDefaultableParams{ + UseStrictSecurity: ptr.To(false), + }, + RetentionPeriod: "1", + RemovePvcAfterDelete: true, + StorageDataPath: "/tmp/", + }, + }, + func(cr *vmv1beta1.VMSingle) { + createdChildObjects := types.NamespacedName{Namespace: namespace, Name: cr.PrefixedName()} + var createdDeploy appsv1.Deployment + Expect(k8sClient.Get(ctx, createdChildObjects, &createdDeploy)).To(Succeed()) + ts := createdDeploy.Spec.Template.Spec + Expect(ts.Containers).To(HaveLen(1)) + Expect(ts.Volumes).To(HaveLen(1)) + Expect(ts.Containers[0].VolumeMounts).To(HaveLen(1)) }), Entry("with external volume", "externalvolume", true, &vmv1beta1.VMSingle{ @@ -292,7 +318,6 @@ var _ = Describe("test vmsingle Controller", Label("vm", "single"), func() { RetentionPeriod: "1", RemovePvcAfterDelete: true, StorageDataPath: "/custom-path/internal/dir", - Storage: &corev1.PersistentVolumeClaimSpec{}, VMBackup: &vmv1beta1.VMBackup{ Destination: "fs:///opt/backup", VolumeMounts: []corev1.VolumeMount{{Name: "backup", MountPath: "/opt/backup"}}, @@ -307,7 +332,8 @@ var _ = Describe("test vmsingle Controller", Label("vm", "single"), func() { Expect(ts.Containers).To(HaveLen(2)) Expect(ts.Volumes).To(HaveLen(4)) Expect(ts.Containers[0].VolumeMounts).To(HaveLen(3)) - Expect(ts.Containers[0].VolumeMounts[0].Name).To(Equal("data")) + Expect(ts.Containers[0].VolumeMounts[0].Name).To(Equal("unused")) + Expect(ts.Containers[0].VolumeMounts[1].Name).To(Equal("data")) Expect(ts.Containers[1].VolumeMounts).To(HaveLen(3)) Expect(ts.Containers[1].VolumeMounts[0].Name).To(Equal("data")) Expect(ts.Containers[1].VolumeMounts[1].Name).To(Equal("backup"))