From 6c658264cfaf8959c87dca919c79bcecf17681cb Mon Sep 17 00:00:00 2001 From: Adam Boguszewski Date: Thu, 7 Sep 2023 11:46:16 +0200 Subject: [PATCH] feat!: truncate fullname after 22 characters --- .changelog/3248.breaking.txt | 1 + deploy/helm/sumologic/README.md | 2 +- deploy/helm/sumologic/templates/NOTES.txt | 4 --- .../sumologic/templates/_helpers/_common.tpl | 7 +++--- docs/v4-migration-doc.md | 8 +++++- tests/helm/common_test.go | 25 ------------------- tests/helm/const.go | 4 +-- tests/helm/testdata/everything-enabled.yaml | 7 ++++++ tests/integration/internal/strings/strings.go | 6 ++++- 9 files changed, 27 insertions(+), 37 deletions(-) create mode 100644 .changelog/3248.breaking.txt diff --git a/.changelog/3248.breaking.txt b/.changelog/3248.breaking.txt new file mode 100644 index 0000000000..44171f1433 --- /dev/null +++ b/.changelog/3248.breaking.txt @@ -0,0 +1 @@ +feat!: truncate fullname after 22 characters \ No newline at end of file diff --git a/deploy/helm/sumologic/README.md b/deploy/helm/sumologic/README.md index d8a4266dfd..c34bb655df 100644 --- a/deploy/helm/sumologic/README.md +++ b/deploy/helm/sumologic/README.md @@ -21,7 +21,7 @@ The following table lists the configurable parameters of the Sumo Logic chart an | Parameter | Description | Default | | ------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | | `nameOverride` | Used to override the Chart name. | `Nil` | -| `fullnameOverride` | Used to override the chart's full name. | `Nil` | +| `fullnameOverride` | Used to override the chart's full name. Names longer than 22 characters will be truncated. | `Nil` | | `namespaceOverride` | Used to override the chart's default target namepace. | `Nil` | | `sumologic.setupEnabled` | If enabled, a pre-install hook will create Collector and Sources in Sumo Logic. | `true` | | `sumologic.cleanupEnabled` | If enabled, a pre-delete hook will destroy Kubernetes secret and Sumo Logic Collector. | `false` | diff --git a/deploy/helm/sumologic/templates/NOTES.txt b/deploy/helm/sumologic/templates/NOTES.txt index 5dc16f2e4e..a22070d1f7 100644 --- a/deploy/helm/sumologic/templates/NOTES.txt +++ b/deploy/helm/sumologic/templates/NOTES.txt @@ -4,10 +4,6 @@ Thank you for installing {{ .Chart.Name }}. WARNING: You defined sumologic.clusterName with spaces, which is not supported. Spaces have been replaced with dashes. {{- end }} -{{- if gt (len .Release.Name) 19 }} -WARNING: Your release name is longer than 19 characters. Some features may not work due length limits on Kubernetes labels. This limit will be enforced explicitly in the next major release. -{{- end }} - A Collector with the name {{ .Values.sumologic.collectorName | default (include "sumologic.clusterNameReplaceSpaceWithDash" . ) | quote }} has been created in your Sumo Logic account. Check the release status by running: diff --git a/deploy/helm/sumologic/templates/_helpers/_common.tpl b/deploy/helm/sumologic/templates/_helpers/_common.tpl index 190dda595e..1c06e0448e 100644 --- a/deploy/helm/sumologic/templates/_helpers/_common.tpl +++ b/deploy/helm/sumologic/templates/_helpers/_common.tpl @@ -7,14 +7,15 @@ Expand the name of the chart. {{/* Create a default fully qualified app name. -We truncate at 63 chars because some Kubernetes name fields are limited to this (by the DNS naming spec). +We truncate at 22 chars because some Kubernetes name fields are limited to 63 characters (by the DNS naming spec). +In particular, some statefulsets will have too long names if the name is longer than 22 characters. */}} {{- define "sumologic.fullname" -}} {{- if .Values.fullnameOverride }} -{{- .Values.fullnameOverride | trunc 63 | trimSuffix "-" }} +{{- .Values.fullnameOverride | trunc 22 | trimSuffix "-" }} {{- else }} {{- $name := default .Chart.Name .Values.nameOverride -}} -{{- printf "%s-%s" .Release.Name $name | trunc 63 | trimSuffix "-" }} +{{- printf "%s-%s" .Release.Name $name | trunc 22 | trimSuffix "-" }} {{- end -}} {{- end -}} diff --git a/docs/v4-migration-doc.md b/docs/v4-migration-doc.md index 07497730b7..edcfbc96b4 100644 --- a/docs/v4-migration-doc.md +++ b/docs/v4-migration-doc.md @@ -2,9 +2,10 @@ -- [Kubernetes Collection v4.0.0 - Breaking Changes](#kubernetes-collection-v400---breaking-changes) +- [Kubernetes Collection `v4.0.0` - Breaking Changes](#kubernetes-collection-v400---breaking-changes) - [Important changes](#important-changes) - [OpenTelemetry Collector](#opentelemetry-collector) + - [Drop Prometheus recording rule metrics](#drop-prometheus-recording-rule-metrics) - [How to upgrade](#how-to-upgrade) - [Requirements](#requirements) - [Metrics migration](#metrics-migration) @@ -156,3 +157,8 @@ require additional action. - node_network_transmit_bytes_total - node_filesystem_avail_bytes - node_filesystem_size_bytes + +- Truncating full name to 22 characters + + Some Kubernetes objects, for example statefulsets, have a tight (63 characters) limit for their names. Because of that, we truncate the + prefix that is attached to the names. In particular, the value under key `fullnameOverride` will be truncated to 22 characters. diff --git a/tests/helm/common_test.go b/tests/helm/common_test.go index e68d9f4824..4dabd30494 100644 --- a/tests/helm/common_test.go +++ b/tests/helm/common_test.go @@ -172,31 +172,6 @@ func TestNameAndLabelLength(t *testing.T) { } } -func TestReleaseNameTooLong(t *testing.T) { - valuesFilePath := path.Join(testDataDirectory, "everything-enabled.yaml") - releaseName := strings.Repeat("a", maxHelmReleaseNameLength+1) - _, err := RenderTemplateE( - t, - &helm.Options{ - ValuesFiles: []string{valuesFilePath}, - SetStrValues: map[string]string{ - "sumologic.accessId": "accessId", - "sumologic.accessKey": "accessKey", - }, - Logger: logger.Discard, // the log output is noisy and doesn't help much - }, - chartDirectory, - releaseName, - []string{}, - true, - "--namespace", - defaultNamespace, - ) - require.NoError(t, err) - // we can't check whether NOTES are rendered correctly due to https://github.com/helm/helm/issues/6901 - // TODO: Add an error check here after we start enforcing the limit: https://github.com/SumoLogic/sumologic-kubernetes-collection/issues/3057 -} - // check the built-in labels added to all K8s objects created by the chart func checkBuiltinLabels(t *testing.T, object metav1.Object, chartVersion string) { labels := object.GetLabels() diff --git a/tests/helm/const.go b/tests/helm/const.go index 7317a7663e..f72969a14d 100644 --- a/tests/helm/const.go +++ b/tests/helm/const.go @@ -5,14 +5,14 @@ const ( yamlDirectory = "static" chartDirectory = "../../deploy/helm/sumologic" chartName = "sumologic" - releaseName = "collection-test" + releaseName = "col-test" defaultNamespace = "sumologic" defaultK8sVersion = "1.26.0" testDataDirectory = "./testdata" otelConfigFileName = "config.yaml" otelImageFIPSSuffix = "-fips" otelContainerName = "otelcol" - maxHelmReleaseNameLength = 19 // Helm allows up to 53, this is our own limit + maxHelmReleaseNameLength = 22 // Helm allows up to 53, but for a name longer than 22 some statefulset names will be too long k8sMaxNameLength = 253 // see https://kubernetes.io/docs/concepts/overview/working-with-objects/names/ k8sMaxLabelLength = 63 // see https://kubernetes.io/docs/concepts/overview/working-with-objects/names/ ) diff --git a/tests/helm/testdata/everything-enabled.yaml b/tests/helm/testdata/everything-enabled.yaml index 6652523954..f2107ed870 100644 --- a/tests/helm/testdata/everything-enabled.yaml +++ b/tests/helm/testdata/everything-enabled.yaml @@ -1,3 +1,10 @@ +sumologic: + logs: + collector: + allowSideBySide: true + otelcloudwatch: + enabled: true + metadata: logs: autoscaling: diff --git a/tests/integration/internal/strings/strings.go b/tests/integration/internal/strings/strings.go index 24c2d01ae4..8b08853c42 100644 --- a/tests/integration/internal/strings/strings.go +++ b/tests/integration/internal/strings/strings.go @@ -8,6 +8,10 @@ import ( "time" ) +const ( + maxReleaseNameLength = 12 +) + func NameFromT(t *testing.T) string { return strings.ReplaceAll(strings.ToLower(t.Name()), "_", "-") } @@ -29,5 +33,5 @@ func ValueFileFromT(t *testing.T) string { func ReleaseNameFromT(t *testing.T) string { h := fnv.New32a() h.Write([]byte(t.Name())) - return fmt.Sprintf("rel-%d", h.Sum32()) + return fmt.Sprintf("rel-%d", h.Sum32())[:maxReleaseNameLength] }