Skip to content

Commit

Permalink
chore: use separate label for opt-out
Browse files Browse the repository at this point in the history 
Also: prefix all labels with dash0.com and use kebap case instead of
dot-separated notation for label names.
  • Loading branch information
@basti1302
basti1302 committed May 31, 2024
1 parent 449903f commit 7f3b7f6
Show file tree
Hide file tree
Showing 10 changed files with 94 additions and 106 deletions.
20 changes: 8 additions & 12 deletions internal/controller/dash0_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -665,7 +665,7 @@ func (r *Dash0Reconciler) findAndUninstrumentCronJobs(
logger *logr.Logger,
) error {
matchingWorkloadsInNamespace, err :=
r.ClientSet.BatchV1().CronJobs(namespace).List(ctx, util.WorkloadsWithDash0InstrumentedLabelExlucdingOptOutFilter)
r.ClientSet.BatchV1().CronJobs(namespace).List(ctx, util.WorkloadsWithDash0InstrumentedLabelFilter)
if err != nil {
return fmt.Errorf("error when querying instrumented cron jobs: %w", err)
}
Expand All @@ -688,7 +688,7 @@ func (r *Dash0Reconciler) uninstrumentCronJob(

func (r *Dash0Reconciler) findAndUninstrumentDaemonSets(ctx context.Context, namespace string, logger *logr.Logger) error {
matchingWorkloadsInNamespace, err :=
r.ClientSet.AppsV1().DaemonSets(namespace).List(ctx, util.WorkloadsWithDash0InstrumentedLabelExlucdingOptOutFilter)
r.ClientSet.AppsV1().DaemonSets(namespace).List(ctx, util.WorkloadsWithDash0InstrumentedLabelFilter)
if err != nil {
return fmt.Errorf("error when querying instrumented daemon sets: %w", err)
}
Expand All @@ -715,7 +715,7 @@ func (r *Dash0Reconciler) findAndUninstrumentDeployments(
logger *logr.Logger,
) error {
matchingWorkloadsInNamespace, err :=
r.ClientSet.AppsV1().Deployments(namespace).List(ctx, util.WorkloadsWithDash0InstrumentedLabelExlucdingOptOutFilter)
r.ClientSet.AppsV1().Deployments(namespace).List(ctx, util.WorkloadsWithDash0InstrumentedLabelFilter)
if err != nil {
return fmt.Errorf("error when querying instrumented deployments: %w", err)
}
Expand All @@ -741,7 +741,7 @@ func (r *Dash0Reconciler) findAndHandleJobOnUninstrumentation(
namespace string,
logger *logr.Logger,
) error {
matchingWorkloadsInNamespace, err := r.ClientSet.BatchV1().Jobs(namespace).List(ctx, util.WorkloadsWithDash0InstrumentedLabelExlucdingOptOutFilter)
matchingWorkloadsInNamespace, err := r.ClientSet.BatchV1().Jobs(namespace).List(ctx, util.WorkloadsWithDash0InstrumentedLabelFilter)
if err != nil {
return fmt.Errorf("error when querying instrumented jobs: %w", err)
}
Expand Down Expand Up @@ -787,15 +787,11 @@ func (r *Dash0Reconciler) handleJobOnUninstrumentation(ctx context.Context, job
// We only need remove the labels from that instrumentation attempt to clean up.
newWorkloadModifier(r.Versions, &logger).RemoveLabelsFromImmutableJob(&job)

// Apparently for jobs we do not need to set the "dash0.webhook.ignore.once" label, since changing there
// Apparently for jobs we do not need to set the "dash0.com/webhook-ignore-once" label, since changing their
// labels does not trigger a new admission request.
return r.Client.Update(ctx, &job)
} else if util.HasOptedOutOfInstrumenation(&job.ObjectMeta) {
// There is an opt-out marker for this job, so we have never attempted to instrument it and also do not
// need to revert anything or remove any labels.
return nil
} else {
// No dash0.instrumented label is present, do nothing.
// No dash0.com/instrumented label is present, do nothing.
return nil
}
}, &logger)
Expand Down Expand Up @@ -823,7 +819,7 @@ func (r *Dash0Reconciler) findAndUninstrumentReplicaSets(
logger *logr.Logger,
) error {
matchingWorkloadsInNamespace, err :=
r.ClientSet.AppsV1().ReplicaSets(namespace).List(ctx, util.WorkloadsWithDash0InstrumentedLabelExlucdingOptOutFilter)
r.ClientSet.AppsV1().ReplicaSets(namespace).List(ctx, util.WorkloadsWithDash0InstrumentedLabelFilter)
if err != nil {
return fmt.Errorf("error when querying instrumented replica sets: %w", err)
}
Expand All @@ -850,7 +846,7 @@ func (r *Dash0Reconciler) findAndUninstrumentStatefulSets(
logger *logr.Logger,
) error {
matchingWorkloadsInNamespace, err :=
r.ClientSet.AppsV1().StatefulSets(namespace).List(ctx, util.WorkloadsWithDash0InstrumentedLabelExlucdingOptOutFilter)
r.ClientSet.AppsV1().StatefulSets(namespace).List(ctx, util.WorkloadsWithDash0InstrumentedLabelFilter)
if err != nil {
return fmt.Errorf("error when querying instrumented stateful sets: %w", err)
}
Expand Down
2 changes: 1 addition & 1 deletion internal/controller/dash0_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ var _ = Describe("The Dash0 controller", func() {
triggerReconcileRequest(ctx, reconciler, "Trigger first reconcile request")

name := UniqueName(JobNamePrefix)
By("Create a job with labels (dash0.instrumented=unsuccessful)")
By("Create a job with label dash0.com/instrumented=false")
job := CreateJobForWhichAnInstrumentationAttemptHasFailed(ctx, k8sClient, namespace, name)
createdObjects = append(createdObjects, job)

Expand Down
55 changes: 27 additions & 28 deletions internal/util/labels.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,35 +10,32 @@ import (
)

const (
instrumentedLabelKey = "dash0.instrumented"
instrumentedLabelKey = "dash0.com/instrumented"

// InstrumentedLabelValueSuccessful os written by the operator when then instrumentation attempt has been
// instrumentedLabelValueSuccessful os written by the operator when then instrumentation attempt has been
// successful.
instrumentedLabelValueSuccessful instrumentedState = "successful"
instrumentedLabelValueSuccessful instrumentedState = "true"

// InstrumentedLabelValueUnsuccessful is written by the operator when then instrumentation attempt has failed.
instrumentedLabelValueUnsuccessful instrumentedState = "unsuccessful"
// instrumentedLabelValueUnsuccessful is written by the operator when then instrumentation attempt has failed.
instrumentedLabelValueUnsuccessful instrumentedState = "false"

// InstrumentedLabelValueOptOut is never written by the operator, this can be set externally to opt-out of
// instrumentation for a particular workload
instrumentedLabelValueOptOut instrumentedState = "false"

// InstrumentedLabelValueUnknown should never occur in the wild, it is used as a fallback for inconsistent states
// instrumentedLabelValueUnknown should never occur in the wild, it is used as a fallback for inconsistent states
// or when the label is missing entirely.
instrumentedLabelValueUnknown instrumentedState = "unknown"

operatorVersionLabelKey = "dash0.operator.version"
initContainerImageVersionLabelKey = "dash0.initcontainer.image.version"
instrumentedByLabelKey = "dash0.instrumented.by"
webhookIgnoreOnceLabelKey = "dash0.webhook.ignore.once"
optOutLabelKey = "dash0.com/opt-out"
operatorVersionLabelKey = "dash0.com/operator-version"
initContainerImageVersionLabelKey = "dash0.com/init-container-image-version"
instrumentedByLabelKey = "dash0.com/instrumented-by"
webhookIgnoreOnceLabelKey = "dash0.com/webhook-ignore-once"
)

var (
WorkloadsWithoutDash0InstrumentedLabelFilter = metav1.ListOptions{
LabelSelector: fmt.Sprintf("!%s", instrumentedLabelKey),
LabelSelector: fmt.Sprintf("!%s,%s != true", instrumentedLabelKey, optOutLabelKey),
}
WorkloadsWithDash0InstrumentedLabelExlucdingOptOutFilter = metav1.ListOptions{
LabelSelector: fmt.Sprintf("%[1]s,%[1]s != false", instrumentedLabelKey),
WorkloadsWithDash0InstrumentedLabelFilter = metav1.ListOptions{
LabelSelector: fmt.Sprintf("%s,%s != true", instrumentedLabelKey, optOutLabelKey),
}
)

Expand Down Expand Up @@ -71,9 +68,7 @@ func addLabel(meta *metav1.ObjectMeta, key string, value string) {
}

func RemoveInstrumentationLabels(meta *metav1.ObjectMeta) {
if meta.GetLabels()[instrumentedLabelKey] != "false" {
removeLabel(meta, instrumentedLabelKey)
}
removeLabel(meta, instrumentedLabelKey)
removeLabel(meta, operatorVersionLabelKey)
removeLabel(meta, initContainerImageVersionLabelKey)
removeLabel(meta, instrumentedByLabelKey)
Expand All @@ -91,10 +86,6 @@ func InstrumenationAttemptHasFailed(meta *metav1.ObjectMeta) bool {
return readInstrumentationState(meta) == instrumentedLabelValueUnsuccessful
}

func HasOptedOutOfInstrumenation(meta *metav1.ObjectMeta) bool {
return readInstrumentationState(meta) == instrumentedLabelValueOptOut
}

func readInstrumentationState(meta *metav1.ObjectMeta) instrumentedState {
if meta.Labels == nil {
return instrumentedLabelValueUnknown
Expand All @@ -104,20 +95,28 @@ func readInstrumentationState(meta *metav1.ObjectMeta) instrumentedState {
return instrumentedLabelValueSuccessful
case string(instrumentedLabelValueUnsuccessful):
return instrumentedLabelValueUnsuccessful
case string(instrumentedLabelValueOptOut):
return instrumentedLabelValueOptOut
default:
return instrumentedLabelValueUnknown
}
}

func HasOptedOutOfInstrumenation(meta *metav1.ObjectMeta) bool {
if meta.Labels == nil {
return false
}
if value, ok := meta.Labels[optOutLabelKey]; ok && value == "true" {
return true
}
return false
}

func CheckAndDeleteIgnoreOnceLabel(meta *metav1.ObjectMeta) bool {
if meta.Labels == nil {
return false
}
if value, ok := meta.Labels[webhookIgnoreOnceLabelKey]; ok && value == "true" {
if value, ok := meta.Labels[webhookIgnoreOnceLabelKey]; ok {
delete(meta.Labels, webhookIgnoreOnceLabelKey)
return true
return value == "true"
}
return false
}
14 changes: 7 additions & 7 deletions internal/webhook/dash0_webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func (h *Handler) handleCronJob(
return h.postProcess(request, cronJob, false, true, logger)
}
if util.HasOptedOutOfInstrumenation(&cronJob.ObjectMeta) {
return admission.Allowed("not instrumenting this resource due to dash0.instrumented=false (opt-out)")
return admission.Allowed("not instrumenting this resource due to dash0.com/opt-out=true")
}
hasBeenModified := h.newWorkloadModifier(logger).ModifyCronJob(cronJob)
return h.postProcess(request, cronJob, hasBeenModified, false, logger)
Expand All @@ -133,7 +133,7 @@ func (h *Handler) handleDaemonSet(
return h.postProcess(request, daemonSet, false, true, logger)
}
if util.HasOptedOutOfInstrumenation(&daemonSet.ObjectMeta) {
return admission.Allowed("not instrumenting this resource due to dash0.instrumented=false (opt-out)")
return admission.Allowed("not instrumenting this resource due to dash0.com/opt-out=true")
}
hasBeenModified := h.newWorkloadModifier(logger).ModifyDaemonSet(daemonSet)
return h.postProcess(request, daemonSet, hasBeenModified, false, logger)
Expand All @@ -153,7 +153,7 @@ func (h *Handler) handleDeployment(
return h.postProcess(request, deployment, false, true, logger)
}
if util.HasOptedOutOfInstrumenation(&deployment.ObjectMeta) {
return admission.Allowed("not instrumenting this resource due to dash0.instrumented=false (opt-out)")
return admission.Allowed("not instrumenting this resource due to dash0.com/opt-out=true")
}
hasBeenModified := h.newWorkloadModifier(logger).ModifyDeployment(deployment)
return h.postProcess(request, deployment, hasBeenModified, false, logger)
Expand All @@ -173,7 +173,7 @@ func (h *Handler) handleJob(
return h.postProcess(request, job, false, true, logger)
}
if util.HasOptedOutOfInstrumenation(&job.ObjectMeta) {
return admission.Allowed("not instrumenting this resource due to dash0.instrumented=false (opt-out)")
return admission.Allowed("not instrumenting this resource due to dash0.com/opt-out=true")
}
hasBeenModified := h.newWorkloadModifier(logger).ModifyJob(job)
return h.postProcess(request, job, hasBeenModified, false, logger)
Expand All @@ -193,7 +193,7 @@ func (h *Handler) handleReplicaSet(
return h.postProcess(request, replicaSet, false, true, logger)
}
if util.HasOptedOutOfInstrumenation(&replicaSet.ObjectMeta) {
return admission.Allowed("not instrumenting this resource due to dash0.instrumented=false (opt-out)")
return admission.Allowed("not instrumenting this resource due to dash0.com/opt-out=true")
}
hasBeenModified := h.newWorkloadModifier(logger).ModifyReplicaSet(replicaSet)
return h.postProcess(request, replicaSet, hasBeenModified, false, logger)
Expand All @@ -213,7 +213,7 @@ func (h *Handler) handleStatefulSet(
return h.postProcess(request, statefulSet, false, true, logger)
}
if util.HasOptedOutOfInstrumenation(&statefulSet.ObjectMeta) {
return admission.Allowed("not instrumenting this resource due to dash0.instrumented=false (opt-out)")
return admission.Allowed("not instrumenting this resource due to dash0.com/out-out=true")
}
hasBeenModified := h.newWorkloadModifier(logger).ModifyStatefulSet(statefulSet)
return h.postProcess(request, statefulSet, hasBeenModified, false, logger)
Expand Down Expand Up @@ -252,7 +252,7 @@ func (h *Handler) postProcess(
}

if ignored {
logger.Info("Ignoring this admission request due to the presence of dash0.webhook.ignore.once")
logger.Info("Ignoring this admission request due to the presence of dash0.com/webhook-ignore-once")
// deliberately not queueing an event for this case
return admission.PatchResponseFromRaw(request.Object.Raw, marshalled)
}
Expand Down
18 changes: 6 additions & 12 deletions internal/webhook/dash0_webhook_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,6 @@ var _ = Describe("The Dash0 webhook", func() {
name := UniqueName(CronJobNamePrefix)
workload := CronJobWithOptOutLabel(TestNamespaceName, name)
createdObjects = append(createdObjects, workload)
AddLabel(&workload.ObjectMeta, "dash0.instrumented", "false")
CreateWorkload(ctx, k8sClient, workload)
workload = GetCronJob(ctx, k8sClient, TestNamespaceName, name)
VerifyCronJobWithOptOutLabel(workload)
Expand All @@ -180,7 +179,6 @@ var _ = Describe("The Dash0 webhook", func() {
name := UniqueName(DaemonSetNamePrefix)
workload := DaemonSetWithOptOutLabel(TestNamespaceName, name)
createdObjects = append(createdObjects, workload)
AddLabel(&workload.ObjectMeta, "dash0.instrumented", "false")
CreateWorkload(ctx, k8sClient, workload)
workload = GetDaemonSet(ctx, k8sClient, TestNamespaceName, name)
VerifyDaemonSetWithOptOutLabel(workload)
Expand All @@ -191,7 +189,6 @@ var _ = Describe("The Dash0 webhook", func() {
name := UniqueName(DeploymentNamePrefix)
workload := DeploymentWithOptOutLabel(TestNamespaceName, name)
createdObjects = append(createdObjects, workload)
AddLabel(&workload.ObjectMeta, "dash0.instrumented", "false")
CreateWorkload(ctx, k8sClient, workload)
workload = GetDeployment(ctx, k8sClient, TestNamespaceName, name)
VerifyDeploymentWithOptOutLabel(workload)
Expand All @@ -202,7 +199,6 @@ var _ = Describe("The Dash0 webhook", func() {
name := UniqueName(JobNamePrefix)
workload := JobWithOptOutLabel(TestNamespaceName, name)
createdObjects = append(createdObjects, workload)
AddLabel(&workload.ObjectMeta, "dash0.instrumented", "false")
CreateWorkload(ctx, k8sClient, workload)
workload = GetJob(ctx, k8sClient, TestNamespaceName, name)
VerifyJobWithOptOutLabel(workload)
Expand All @@ -213,7 +209,6 @@ var _ = Describe("The Dash0 webhook", func() {
name := UniqueName(ReplicaSetNamePrefix)
workload := ReplicaSetWithOptOutLabel(TestNamespaceName, name)
createdObjects = append(createdObjects, workload)
AddLabel(&workload.ObjectMeta, "dash0.instrumented", "false")
CreateWorkload(ctx, k8sClient, workload)
workload = GetReplicaSet(ctx, k8sClient, TestNamespaceName, name)
VerifyReplicaSetWithOptOutLabel(workload)
Expand All @@ -224,7 +219,6 @@ var _ = Describe("The Dash0 webhook", func() {
name := UniqueName(StatefulSetNamePrefix)
workload := StatefulSetWithOptOutLabel(TestNamespaceName, name)
createdObjects = append(createdObjects, workload)
AddLabel(&workload.ObjectMeta, "dash0.instrumented", "false")
CreateWorkload(ctx, k8sClient, workload)
workload = GetStatefulSet(ctx, k8sClient, TestNamespaceName, name)
VerifyStatefulSetWithOptOutLabel(workload)
Expand All @@ -237,7 +231,7 @@ var _ = Describe("The Dash0 webhook", func() {
name := UniqueName(CronJobNamePrefix)
workload := BasicCronJob(TestNamespaceName, name)
createdObjects = append(createdObjects, workload)
AddLabel(&workload.ObjectMeta, "dash0.webhook.ignore.once", "true")
AddLabel(&workload.ObjectMeta, "dash0.com/webhook-ignore-once", "true")
CreateWorkload(ctx, k8sClient, workload)
workload = GetCronJob(ctx, k8sClient, TestNamespaceName, name)
VerifyUnmodifiedCronJob(workload)
Expand All @@ -249,7 +243,7 @@ var _ = Describe("The Dash0 webhook", func() {
name := UniqueName(DaemonSetNamePrefix)
workload := BasicDaemonSet(TestNamespaceName, name)
createdObjects = append(createdObjects, workload)
AddLabel(&workload.ObjectMeta, "dash0.webhook.ignore.once", "true")
AddLabel(&workload.ObjectMeta, "dash0.com/webhook-ignore-once", "true")
CreateWorkload(ctx, k8sClient, workload)
workload = GetDaemonSet(ctx, k8sClient, TestNamespaceName, name)
VerifyUnmodifiedDaemonSet(workload)
Expand All @@ -261,7 +255,7 @@ var _ = Describe("The Dash0 webhook", func() {
name := UniqueName(DeploymentNamePrefix)
workload := BasicDeployment(TestNamespaceName, name)
createdObjects = append(createdObjects, workload)
AddLabel(&workload.ObjectMeta, "dash0.webhook.ignore.once", "true")
AddLabel(&workload.ObjectMeta, "dash0.com/webhook-ignore-once", "true")
CreateWorkload(ctx, k8sClient, workload)
workload = GetDeployment(ctx, k8sClient, TestNamespaceName, name)
VerifyUnmodifiedDeployment(workload)
Expand All @@ -273,7 +267,7 @@ var _ = Describe("The Dash0 webhook", func() {
name := UniqueName(JobNamePrefix)
workload := BasicJob(TestNamespaceName, name)
createdObjects = append(createdObjects, workload)
AddLabel(&workload.ObjectMeta, "dash0.webhook.ignore.once", "true")
AddLabel(&workload.ObjectMeta, "dash0.com/webhook-ignore-once", "true")
CreateWorkload(ctx, k8sClient, workload)
workload = GetJob(ctx, k8sClient, TestNamespaceName, name)
VerifyUnmodifiedJob(workload)
Expand All @@ -285,7 +279,7 @@ var _ = Describe("The Dash0 webhook", func() {
name := UniqueName(ReplicaSetNamePrefix)
workload := BasicReplicaSet(TestNamespaceName, name)
createdObjects = append(createdObjects, workload)
AddLabel(&workload.ObjectMeta, "dash0.webhook.ignore.once", "true")
AddLabel(&workload.ObjectMeta, "dash0.com/webhook-ignore-once", "true")
CreateWorkload(ctx, k8sClient, workload)
workload = GetReplicaSet(ctx, k8sClient, TestNamespaceName, name)
VerifyUnmodifiedReplicaSet(workload)
Expand All @@ -297,7 +291,7 @@ var _ = Describe("The Dash0 webhook", func() {
name := UniqueName(StatefulSetNamePrefix)
workload := BasicStatefulSet(TestNamespaceName, name)
createdObjects = append(createdObjects, workload)
AddLabel(&workload.ObjectMeta, "dash0.webhook.ignore.once", "true")
AddLabel(&workload.ObjectMeta, "dash0.com/webhook-ignore-once", "true")
CreateWorkload(ctx, k8sClient, workload)
workload = GetStatefulSet(ctx, k8sClient, TestNamespaceName, name)
VerifyUnmodifiedStatefulSet(workload)
Expand Down
2 changes: 1 addition & 1 deletion test-resources/collector/e2e.values.yaml.template
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ mode: daemonset
service:
enabled: true
additionalLabels:
dash0.instrumented: "false"
dash0.com/opt-out: "true"
resources:
limits:
memory: 500Mi
Expand Down
2 changes: 1 addition & 1 deletion test-resources/collector/manual.values.yaml.template
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ mode: daemonset
service:
enabled: true
additionalLabels:
dash0.instrumented: "false"
dash0.com/opt-out: "true"
resources:
limits:
memory: 500Mi
Expand Down
Loading

0 comments on commit 7f3b7f6

Please sign in to comment.