Skip to content

CP/DP Split: fix label updates #3370

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

Merged
merged 2 commits into from
May 14, 2025
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
2 changes: 1 addition & 1 deletion build/Dockerfile.nginx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ WORKDIR /tmp
RUN apk add --no-cache git make \
&& git clone https://github.com/nginx/agent.git \
&& cd agent \
&& git checkout v3 \
&& git checkout e745a3236e0f02a579461a5a435b3bcd410a686c \
&& make build

FROM nginx:1.28.0-alpine-otel
Expand Down
2 changes: 1 addition & 1 deletion build/Dockerfile.nginxplus
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ WORKDIR /tmp
RUN apk add --no-cache git make \
&& git clone https://github.com/nginx/agent.git \
&& cd agent \
&& git checkout v3 \
&& git checkout e745a3236e0f02a579461a5a435b3bcd410a686c \
&& make build

FROM alpine:3.21
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ require (
github.com/go-logr/logr v1.4.2
github.com/google/go-cmp v0.7.0
github.com/google/uuid v1.6.0
github.com/nginx/agent/v3 v3.0.0-20250429163223-735f50381a9e
github.com/nginx/agent/v3 v3.0.0-20250513105855-e745a3236e0f
github.com/nginx/telemetry-exporter v0.1.4
github.com/onsi/ginkgo/v2 v2.23.4
github.com/onsi/gomega v1.37.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,8 @@ github.com/morikuni/aec v1.0.0 h1:nP9CBfwrvYnBRgY6qfDQkygYDmYwOilePFkwzv4dU8A=
github.com/morikuni/aec v1.0.0/go.mod h1:BbKIizmSmc5MMPqRYbxO4ZU0S0+P200+tUnFx7PXmsc=
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq1c1nUAm88MOHcQC9l5mIlSMApZMrHA=
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ=
github.com/nginx/agent/v3 v3.0.0-20250429163223-735f50381a9e h1:Cw/fGXymS9ytwusxE7TaySDovKH+yQuWRI0vLJ4rJxU=
github.com/nginx/agent/v3 v3.0.0-20250429163223-735f50381a9e/go.mod h1:O/31aKtii/mpiZmFGMcTNDoLtKzwTyTXOBMSRkMaPvs=
github.com/nginx/agent/v3 v3.0.0-20250513105855-e745a3236e0f h1:fSUAaR1AxmmbmGMRkvKGY2+LhuVpBp7tbBFLLgDMjNQ=
github.com/nginx/agent/v3 v3.0.0-20250513105855-e745a3236e0f/go.mod h1:O/31aKtii/mpiZmFGMcTNDoLtKzwTyTXOBMSRkMaPvs=
github.com/nginx/telemetry-exporter v0.1.4 h1:3ikgKlyz/O57oaBLkxCInMjr74AhGTKr9rHdRAkkl/w=
github.com/nginx/telemetry-exporter v0.1.4/go.mod h1:bl6qmsxgk4a9D0X8R5E3sUNXN2iECPEK1JNbRLhN5C4=
github.com/nginxinc/nginx-plus-go-client/v2 v2.0.1 h1:5VVK38bnELMDWnwfF6dSv57ResXh9AUzeDa72ENj94o=
Expand Down
2 changes: 1 addition & 1 deletion internal/mode/static/provisioner/provisioner.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ func (p *NginxProvisioner) provisionNginx(
}

// if agent configmap was updated, then we'll need to restart the deployment
if agentConfigMapUpdated && !deploymentCreated {
if agentConfigMapUpdated && !deploymentCreated && deploymentObj != nil {
updateCtx, cancel := context.WithTimeout(ctx, 30*time.Second)
defer cancel()

Expand Down
79 changes: 67 additions & 12 deletions internal/mode/static/provisioner/setter.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
package provisioner

import (
"maps"

appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
)
Expand All @@ -12,54 +15,103 @@
func objectSpecSetter(object client.Object) controllerutil.MutateFn {
switch obj := object.(type) {
case *appsv1.Deployment:
return deploymentSpecSetter(obj, obj.Spec)
return deploymentSpecSetter(obj, obj.Spec, obj.ObjectMeta)
case *corev1.Service:
return serviceSpecSetter(obj, obj.Spec)
return serviceSpecSetter(obj, obj.Spec, obj.ObjectMeta)
case *corev1.ServiceAccount:
return func() error { return nil }
return serviceAccountSpecSetter(obj, obj.ObjectMeta)
case *corev1.ConfigMap:
return configMapSpecSetter(obj, obj.Data)
return configMapSpecSetter(obj, obj.Data, obj.ObjectMeta)
case *corev1.Secret:
return secretSpecSetter(obj, obj.Data)
return secretSpecSetter(obj, obj.Data, obj.ObjectMeta)
case *rbacv1.Role:
return roleSpecSetter(obj, obj.Rules)
return roleSpecSetter(obj, obj.Rules, obj.ObjectMeta)

Check warning on line 28 in internal/mode/static/provisioner/setter.go

View check run for this annotation

Codecov / codecov/patch

internal/mode/static/provisioner/setter.go#L28

Added line #L28 was not covered by tests
case *rbacv1.RoleBinding:
return roleBindingSpecSetter(obj, obj.RoleRef, obj.Subjects)
return roleBindingSpecSetter(obj, obj.RoleRef, obj.Subjects, obj.ObjectMeta)

Check warning on line 30 in internal/mode/static/provisioner/setter.go

View check run for this annotation

Codecov / codecov/patch

internal/mode/static/provisioner/setter.go#L30

Added line #L30 was not covered by tests
}

return nil
}

func deploymentSpecSetter(deployment *appsv1.Deployment, spec appsv1.DeploymentSpec) controllerutil.MutateFn {
func deploymentSpecSetter(
deployment *appsv1.Deployment,
spec appsv1.DeploymentSpec,
objectMeta metav1.ObjectMeta,
) controllerutil.MutateFn {
return func() error {
deployment.Labels = objectMeta.Labels
deployment.Annotations = objectMeta.Annotations
deployment.Spec = spec
return nil
}
}

func serviceSpecSetter(service *corev1.Service, spec corev1.ServiceSpec) controllerutil.MutateFn {
func serviceSpecSetter(
service *corev1.Service,
spec corev1.ServiceSpec,
objectMeta metav1.ObjectMeta,
) controllerutil.MutateFn {
return func() error {
service.Labels = objectMeta.Labels
service.Annotations = objectMeta.Annotations
service.Spec = spec
return nil
}
}

func configMapSpecSetter(configMap *corev1.ConfigMap, data map[string]string) controllerutil.MutateFn {
func serviceAccountSpecSetter(
serviceAccount *corev1.ServiceAccount,
objectMeta metav1.ObjectMeta,
) controllerutil.MutateFn {
return func() error {
serviceAccount.Labels = objectMeta.Labels
serviceAccount.Annotations = objectMeta.Annotations
return nil
}
}

func configMapSpecSetter(
configMap *corev1.ConfigMap,
data map[string]string,
objectMeta metav1.ObjectMeta,
) controllerutil.MutateFn {
return func() error {
// this check ensures we don't trigger an unnecessary update to the agent ConfigMap
// and trigger a Deployment restart
if maps.Equal(configMap.Labels, objectMeta.Labels) &&
maps.Equal(configMap.Annotations, objectMeta.Annotations) &&
maps.Equal(configMap.Data, data) {
return nil
}

configMap.Labels = objectMeta.Labels
configMap.Annotations = objectMeta.Annotations
configMap.Data = data
return nil
}
}

func secretSpecSetter(secret *corev1.Secret, data map[string][]byte) controllerutil.MutateFn {
func secretSpecSetter(
secret *corev1.Secret,
data map[string][]byte,
objectMeta metav1.ObjectMeta,
) controllerutil.MutateFn {
return func() error {
secret.Labels = objectMeta.Labels
secret.Annotations = objectMeta.Annotations
secret.Data = data
return nil
}
}

func roleSpecSetter(role *rbacv1.Role, rules []rbacv1.PolicyRule) controllerutil.MutateFn {
func roleSpecSetter(
role *rbacv1.Role,
rules []rbacv1.PolicyRule,
objectMeta metav1.ObjectMeta,
) controllerutil.MutateFn {

Check warning on line 111 in internal/mode/static/provisioner/setter.go

View check run for this annotation

Codecov / codecov/patch

internal/mode/static/provisioner/setter.go#L111

Added line #L111 was not covered by tests
return func() error {
role.Labels = objectMeta.Labels
role.Annotations = objectMeta.Annotations

Check warning on line 114 in internal/mode/static/provisioner/setter.go

View check run for this annotation

Codecov / codecov/patch

internal/mode/static/provisioner/setter.go#L113-L114

Added lines #L113 - L114 were not covered by tests
role.Rules = rules
return nil
}
Expand All @@ -69,8 +121,11 @@
roleBinding *rbacv1.RoleBinding,
roleRef rbacv1.RoleRef,
subjects []rbacv1.Subject,
objectMeta metav1.ObjectMeta,
) controllerutil.MutateFn {
return func() error {
roleBinding.Labels = objectMeta.Labels
roleBinding.Annotations = objectMeta.Annotations

Check warning on line 128 in internal/mode/static/provisioner/setter.go

View check run for this annotation

Codecov / codecov/patch

internal/mode/static/provisioner/setter.go#L127-L128

Added lines #L127 - L128 were not covered by tests
roleBinding.RoleRef = roleRef
roleBinding.Subjects = subjects
return nil
Expand Down
1 change: 0 additions & 1 deletion internal/mode/static/provisioner/templates.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ allowed_directories:
- /usr/share/nginx
- /var/run/nginx
features:
- connection
- configuration
- certificates
{{- if .EnableMetrics }}
Expand Down
32 changes: 0 additions & 32 deletions internal/mode/static/state/conditions/conditions.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,6 @@ const (
// invalid. Used with ResolvedRefs (false).
RouteReasonInvalidFilter v1.RouteConditionReason = "InvalidFilter"

// GatewayReasonGatewayConflict indicates there are multiple Gateway resources to choose from,
// and we ignored the resource in question and picked another Gateway as the winner.
// This reason is used with GatewayConditionAccepted (false).
GatewayReasonGatewayConflict v1.GatewayConditionReason = "GatewayConflict"

// GatewayMessageGatewayConflict is a message that describes GatewayReasonGatewayConflict.
GatewayMessageGatewayConflict = "The resource is ignored due to a conflicting Gateway resource"

// GatewayReasonUnsupportedValue is used with GatewayConditionAccepted (false) when a value of a field in a Gateway
// is invalid or not supported.
GatewayReasonUnsupportedValue v1.GatewayConditionReason = "UnsupportedValue"
Expand Down Expand Up @@ -574,19 +566,6 @@ func NewGatewayAccepted() conditions.Condition {
}
}

// NewGatewayConflict returns Conditions that indicate the Gateway has a conflict with another Gateway.
func NewGatewayConflict() []conditions.Condition {
return []conditions.Condition{
{
Type: string(v1.GatewayConditionAccepted),
Status: metav1.ConditionFalse,
Reason: string(GatewayReasonGatewayConflict),
Message: GatewayMessageGatewayConflict,
},
NewGatewayConflictNotProgrammed(),
}
}

// NewGatewayAcceptedListenersNotValid returns a Condition that indicates the Gateway is accepted,
// but has at least one listener that is invalid.
func NewGatewayAcceptedListenersNotValid() conditions.Condition {
Expand Down Expand Up @@ -668,17 +647,6 @@ func NewGatewayNotProgrammedInvalid(msg string) conditions.Condition {
}
}

// NewGatewayConflictNotProgrammed returns a custom Programmed Condition that indicates the Gateway has a
// conflict with another Gateway.
func NewGatewayConflictNotProgrammed() conditions.Condition {
return conditions.Condition{
Type: string(v1.GatewayConditionProgrammed),
Status: metav1.ConditionFalse,
Reason: string(GatewayReasonGatewayConflict),
Message: GatewayMessageGatewayConflict,
}
}

// NewNginxGatewayValid returns a Condition that indicates that the NginxGateway config is valid.
func NewNginxGatewayValid() conditions.Condition {
return conditions.Condition{
Expand Down
Loading