Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Makes the deprecated feature-flag warning more precise #723

Merged
merged 4 commits into from
Apr 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@ metadata:
helm.sh/hook: post-install,post-upgrade
{{- end }}
{{- if or (and (.Values.hostMonitoring).enabled (.Values.hostMonitoring).disableReadOnly) (and (.Values.cloudNativeFullStack).enabled (.Values.cloudNativeFullStack).disableReadOnly) }}
operator.dynatrace.com/feature-disable-oneagent-readonly-host-fs: "true"
feature.dynatrace.com/disable-oneagent-readonly-host-fs: "true"
{{- end }}
{{- if (.Values.activeGate).apparmor }}
alpha.operator.dynatrace.com/feature-activegate-apparmor: "true"
feature.dynatrace.com/activegate-apparmor: "true"
{{- end }}
{{- if (.Values.activeGate).readOnlyFs }}
alpha.operator.dynatrace.com/feature-activegate-readonly-fs: "true"
feature.dynatrace.com/activegate-readonly-fs: "true"
{{- end }}
name: {{ .Values.name }}
namespace: {{ .Release.Namespace }}
Expand Down
14 changes: 8 additions & 6 deletions src/api/v1beta1/feature_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,19 @@ import (
"encoding/json"
"fmt"
"strconv"
"strings"

"github.com/Dynatrace/dynatrace-operator/src/logger"
)

const (
DeprecatedFeatureFlagPrefix = "alpha."
DeprecatedFeatureFlagPrefix = "alpha.operator.dynatrace.com/feature-"

FeatureFlagAnnotationBase = "operator.dynatrace.com/"
AnnotationFeaturePrefix = FeatureFlagAnnotationBase + "feature-"
AnnotationFeaturePrefix = "feature.dynatrace.com/"

// activeGate
AnnotationFeatureDisableActiveGateUpdates = AnnotationFeaturePrefix + "disable-activegate-updates"
AnnotationFeatureDisableActivegateRawImage = AnnotationFeaturePrefix + "disable-activegate-raw-image"
AnnotationFeatureDisableActiveGateRawImage = AnnotationFeaturePrefix + "disable-activegate-raw-image"
AnnotationFeatureActiveGateAppArmor = AnnotationFeaturePrefix + "activegate-apparmor"
AnnotationFeatureActiveGateReadOnlyFilesystem = AnnotationFeaturePrefix + "activegate-readonly-fs"
AnnotationFeatureAutomaticKubernetesApiMonitoring = AnnotationFeaturePrefix + "automatic-kubernetes-api-monitoring"
Expand Down Expand Up @@ -162,7 +162,7 @@ func (dk *DynaKube) FeatureDisableReadOnlyOneAgent() bool {
// instead of using embedded ones in the image
// Defaults to false
func (dk *DynaKube) FeatureDisableActivegateRawImage() bool {
return dk.getFeatureFlagRaw(AnnotationFeatureDisableActivegateRawImage) == "true"
return dk.getFeatureFlagRaw(AnnotationFeatureDisableActiveGateRawImage) == "true"
}

// FeatureEnableMultipleOsAgentsOnNode is a feature flag to enable multiple osagents running on the same host
Expand All @@ -184,7 +184,9 @@ func (dk *DynaKube) getFeatureFlagRaw(annotation string) string {
if raw, ok := dk.Annotations[annotation]; ok {
return raw
}
if raw, ok := dk.Annotations[DeprecatedFeatureFlagPrefix+annotation]; ok {
split := strings.Split(annotation, "/")
postFix := split[1]
if raw, ok := dk.Annotations[DeprecatedFeatureFlagPrefix+postFix]; ok {
return raw
}
return ""
Expand Down
1 change: 0 additions & 1 deletion src/api/v1beta1/feature_flags_resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ func (dk *DynaKube) FeatureStatsdResourcesLimits(resourceName corev1.ResourceNam
return statsdResourceRequirements(dk, "limits-"+resourceName)
}

// E.g. "operator.dynatrace.com/feature-activegate-eec-resources-limits-cpu": "100m"
0sewa0 marked this conversation as resolved.
Show resolved Hide resolved
func formatResourceName(resourceName corev1.ResourceName) string {
return "resources-" + string(resourceName)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

const extensionsRuntimeProperties = dynatracev1beta1.FeatureFlagAnnotationBase + "extensions."
const extensionsRuntimeProperties = dynatracev1beta1.AnnotationFeaturePrefix + "extensions."

type EecRuntimeConfig struct {
Revision int `json:"revision"`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ func testBuildDynaKubeWithAnnotations(instanceName string, statsdEnabled bool, a
func TestCreateEecConfigMap(t *testing.T) {
t.Run("happy path", func(t *testing.T) {
instance := testBuildDynaKubeWithAnnotations("dynakube", true, map[string]string{
"operator.dynatrace.com/extensions.debugExtensionDSstatsddisablenamedalivesignals": "false",
"operator.dynatrace.com/extensions.debugExtensionDSstatsdlogoutboundminttraffic": "true",
"operator.dynatrace.com/extensions.debugExtensionDSstatsdcustomloglevel": "trace",
dynatracev1beta1.AnnotationFeaturePrefix + "extensions.debugExtensionDSstatsddisablenamedalivesignals": "false",
dynatracev1beta1.AnnotationFeaturePrefix + "extensions.debugExtensionDSstatsdlogoutboundminttraffic": "true",
dynatracev1beta1.AnnotationFeaturePrefix + "extensions.debugExtensionDSstatsdcustomloglevel": "trace",
})
runtimeConfig := NewEecRuntimeConfig()

Expand All @@ -59,8 +59,8 @@ func TestCreateEecConfigMap(t *testing.T) {

t.Run("no valid EEC runtime properties, StatsD enabled", func(t *testing.T) {
instance := testBuildDynaKubeWithAnnotations("dynakube", true, map[string]string{
"operator.dynatrace.com/debugExtensionDSstatsdlogoutboundminttraffic": "true",
"debugExtensionDSstatsdcustomloglevel": "info",
dynatracev1beta1.AnnotationFeaturePrefix + "debugExtensionDSstatsdlogoutboundminttraffic": "true",
"debugExtensionDSstatsdcustomloglevel": "info",
})
runtimeConfig := NewEecRuntimeConfig()

Expand All @@ -80,7 +80,7 @@ func TestCreateEecConfigMap(t *testing.T) {

t.Run("valid EEC runtime properties but StatsD disabled", func(t *testing.T) {
instance := testBuildDynaKubeWithAnnotations("dynakube", false, map[string]string{
"operator.dynatrace.com/extensions.debugExtensiondummylongflag": "17",
dynatracev1beta1.AnnotationFeaturePrefix + "extensions.debugExtensiondummylongflag": "17",
})
runtimeConfig := NewEecRuntimeConfig()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func TestExtensionController_BuildContainerAndVolumes(t *testing.T) {

t.Run("resource requirements from feature flags", func(t *testing.T) {
stsProperties := testBuildStsProperties()
stsProperties.ObjectMeta.Annotations["operator.dynatrace.com/feature-activegate-eec-resources-limits-cpu"] = "200m"
stsProperties.ObjectMeta.Annotations[dynatracev1beta1.AnnotationFeaturePrefix+"activegate-eec-resources-limits-cpu"] = "200m"
eec := NewExtensionController(stsProperties)

container := eec.BuildContainer()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package statefulset
import (
"testing"

dynatracev1beta1 "github.com/Dynatrace/dynatrace-operator/src/api/v1beta1"
"github.com/Dynatrace/dynatrace-operator/src/controllers/activegate/capability"
"github.com/Dynatrace/dynatrace-operator/src/kubeobjects"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -77,7 +78,7 @@ func TestStatsd_BuildContainerAndVolumes(t *testing.T) {

t.Run("resource requirements from feature flags", func(t *testing.T) {
stsProperties := testBuildStsProperties()
stsProperties.ObjectMeta.Annotations["operator.dynatrace.com/feature-activegate-statsd-resources-requests-memory"] = "500M"
stsProperties.ObjectMeta.Annotations[dynatracev1beta1.AnnotationFeaturePrefix+"activegate-statsd-resources-requests-memory"] = "500M"
statsd := NewStatsd(stsProperties)

container := statsd.BuildContainer()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ func TestStatefulSet_Volumes(t *testing.T) {
})
t.Run(`with FeatureDisableActivegateRawImage=false`, func(t *testing.T) {
instanceRawImg := instance.DeepCopy()
instanceRawImg.Annotations[dynatracev1beta1.AnnotationFeatureDisableActivegateRawImage] = "false"
instanceRawImg.Annotations[dynatracev1beta1.AnnotationFeatureDisableActiveGateRawImage] = "false"

stsProperties := NewStatefulSetProperties(instanceRawImg, capabilityProperties,
"", "", "", "", "",
Expand Down Expand Up @@ -397,7 +397,7 @@ func TestStatefulSet_Env(t *testing.T) {

t.Run(`with FeatureDisableActivegateRawImage=true`, func(t *testing.T) {
instanceRawImg := instance.DeepCopy()
instanceRawImg.Annotations[dynatracev1beta1.AnnotationFeatureDisableActivegateRawImage] = "true"
instanceRawImg.Annotations[dynatracev1beta1.AnnotationFeatureDisableActiveGateRawImage] = "true"

envVars := buildEnvs(NewStatefulSetProperties(instanceRawImg, capabilityProperties,
testUID, "", testComponentFeature, "MSGrouter", "",
Expand Down
2 changes: 1 addition & 1 deletion src/controllers/dynakube/dtclient_reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ func TestReconcileDynatraceClient_TokenValidation(t *testing.T) {
t.Run("API token has missing scope for automatic kubernetes api monitoring", func(t *testing.T) {
dk := base.DeepCopy()
dk.Annotations = map[string]string{
"operator.dynatrace.com/feature-automatic-kubernetes-api-monitoring": "true",
dynatracev1beta1.AnnotationFeatureAutomaticKubernetesApiMonitoring: "true",
}
dk.Spec.ActiveGate = dynatracev1beta1.ActiveGateSpec{
Capabilities: []dynatracev1beta1.CapabilityDisplayName{dynatracev1beta1.KubeMonCapability.DisplayName},
Expand Down
2 changes: 1 addition & 1 deletion src/controllers/dynakube/dynakube_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ func TestReconcileActiveGate_Reconcile(t *testing.T) {
Name: testName,
Namespace: testNamespace,
Annotations: map[string]string{
"operator.dynatrace.com/feature-automatic-kubernetes-api-monitoring": "true",
dynatracev1beta1.AnnotationFeatureAutomaticKubernetesApiMonitoring: "true",
},
},
Spec: dynatracev1beta1.DynaKubeSpec{
Expand Down
3 changes: 2 additions & 1 deletion src/mapper/dynakube_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"testing"

dynatracev1beta1 "github.com/Dynatrace/dynatrace-operator/src/api/v1beta1"
"github.com/Dynatrace/dynatrace-operator/src/scheme/fake"
"github.com/stretchr/testify/assert"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -112,7 +113,7 @@ func TestMapFromDynakube(t *testing.T) {
t.Run("ComponentFeature flag for monitoring system namespaces", func(t *testing.T) {
dk := createTestDynakubeWithMultipleFeatures("appMonitoring", nil, nil)
dk.Annotations = map[string]string{
"operator.dynatrace.com/feature-ignored-namespaces": "[]",
dynatracev1beta1.AnnotationFeatureIgnoredNamespaces: "[]",
}
namespace := createNamespace("openshift-something", nil)
clt := fake.NewClient(dk, namespace)
Expand Down
2 changes: 1 addition & 1 deletion src/mapper/namespaces_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ func TestMapFromNamespace(t *testing.T) {
t.Run("ComponentFeature flag for monitoring system namespaces", func(t *testing.T) {
dk := createTestDynakubeWithMultipleFeatures("appMonitoring", nil, nil)
dk.Annotations = map[string]string{
"operator.dynatrace.com/feature-ignored-namespaces": "[]",
dynatracev1beta1.AnnotationFeatureIgnoredNamespaces: "[]",
}
namespace := createNamespace("openshift-something", nil)
clt := fake.NewClient(dk)
Expand Down
4 changes: 2 additions & 2 deletions src/webhook/validation/deprecation.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ func deprecatedFeatureFlagFormat(dv *dynakubeValidator, dynakube *dynatracev1bet
return ""
}

deprecatedPrefix := dynatracev1beta1.DeprecatedFeatureFlagPrefix + dynatracev1beta1.FeatureFlagAnnotationBase
deprecatedPrefix := dynatracev1beta1.DeprecatedFeatureFlagPrefix
if len(dynatracev1beta1.FlagsWithPrefix(dynakube, deprecatedPrefix)) > 0 {
return fmt.Sprintf(featureDeprecatedWarningMessage, "'alpha.' prefix not necessary")
return fmt.Sprintf(featureDeprecatedWarningMessage, "'alpha.operator.dynatrace.com/feature-' prefix will be replaced with the 'feature.dynatrace.com/' prefix for dynakube feature-flags")
}

return ""
Expand Down
5 changes: 4 additions & 1 deletion src/webhook/validation/deprecation_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package validation

import (
"strings"
"testing"

dynatracev1beta1 "github.com/Dynatrace/dynatrace-operator/src/api/v1beta1"
Expand All @@ -25,8 +26,10 @@ func TestDeprecationWarning(t *testing.T) {

t.Run(`warning present`, func(t *testing.T) {
dynakubeMeta := defaultDynakubeObjectMeta
split := strings.Split(dynatracev1beta1.AnnotationFeatureEnableWebhookReinvocationPolicy, "/")
postFix := split[1]
dynakubeMeta.Annotations = map[string]string{
dynatracev1beta1.DeprecatedFeatureFlagPrefix + dynatracev1beta1.AnnotationFeatureEnableWebhookReinvocationPolicy: "true",
dynatracev1beta1.DeprecatedFeatureFlagPrefix + postFix: "true",
}
dynakube := &dynatracev1beta1.DynaKube{
ObjectMeta: dynakubeMeta,
Expand Down