Skip to content

Configure annotations to be ignored in comparisons during sync #1823

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 12 commits into from
Mar 24, 2022
Merged
Show file tree
Hide file tree
Changes from 7 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
4 changes: 4 additions & 0 deletions charts/postgres-operator/crds/operatorconfigurations.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,10 @@ spec:
enable_sidecars:
type: boolean
default: true
ignored_annotations:
type: array
items:
type: string
infrastructure_roles_secret_name:
type: string
infrastructure_roles_secrets:
Expand Down
7 changes: 7 additions & 0 deletions charts/postgres-operator/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,13 @@ configKubernetes:
enable_pod_disruption_budget: true
# enables sidecar containers to run alongside Spilo in the same pod
enable_sidecars: true

# annotations to be ignored when comparing statefulsets, services etc.
# ignored_annotations:
# - "deployment-time"
# - "k8s.v1.cni.cncf.io/network-status"


# namespaced name of the secret containing infrastructure roles names and passwords
# infrastructure_roles_secret_name: postgresql-infrastructure-roles

Expand Down
3 changes: 2 additions & 1 deletion cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@ package main

import (
"flag"
log "github.com/sirupsen/logrus"
"os"
"os/signal"
"sync"
"syscall"
"time"

log "github.com/sirupsen/logrus"

"github.com/zalando/postgres-operator/pkg/controller"
"github.com/zalando/postgres-operator/pkg/spec"
"github.com/zalando/postgres-operator/pkg/util/k8sutil"
Expand Down
6 changes: 6 additions & 0 deletions docs/reference/operator_parameters.md
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,12 @@ configuration they are grouped under the `kubernetes` key.
Regular expressions like `downscaler/*` etc. are also accepted. Can be used
with [kube-downscaler](https://github.com/hjacobs/kube-downscaler).

* **ignored_annotations**
Some K8s tools inject and update annotations out of the Postgres Operator
control. This can cause rolling updates on each cluster sync cycle. With
this option you can specify an array of annotation keys that should be
ignored when comparing K8s resources on sync. The default is empty.

* **watched_namespace**
The operator watches for Postgres objects in the given namespace. If not
specified, the value is taken from the operator namespace. A special `*`
Expand Down
37 changes: 37 additions & 0 deletions e2e/tests/test_e2e.py
Original file line number Diff line number Diff line change
Expand Up @@ -704,6 +704,43 @@ def test_enable_load_balancer(self):
print('Operator log: {}'.format(k8s.get_operator_log()))
raise

@timeout_decorator.timeout(TEST_TIMEOUT_SEC)
def test_ignored_annotations(self):
'''
Test if injected annotation does not cause failover when listed under ignored_annotations
'''
k8s = self.k8s
cluster_label = 'application=spilo,cluster-name=acid-minimal-cluster'

# get node of current master
master_node, _ = k8s.get_pg_nodes(cluster_label)

patch_config_ignored_annotations = {
"data": {
"ignored_annotations": "deployment-time",
}
}
k8s.update_config(patch_config_ignored_annotations)

pg_crd_annotation = {
"metadata": {
"annotations": {
"deployment-time": "2022-04-01 12:00:00"
},
}
}
k8s.api.custom_objects_api.patch_namespaced_custom_object(
"acid.zalan.do", "v1", "default", "postgresqls", "acid-minimal-cluster", pg_crd_annotation)

annotations = {
"deployment-time": "2022-04-01 12:00:00",
}
self.eventuallyEqual(lambda: k8s.get_operator_state(), {"0": "idle"}, "Operator does not get in sync")
self.eventuallyTrue(lambda: k8s.check_statefulset_annotations(cluster_label, annotations), "Annotations missing")

current_master_node, _ = k8s.get_pg_nodes(cluster_label)
self.eventuallyEqual(lambda: master_node, current_master_node, "unexpected rolling update happened")

@timeout_decorator.timeout(TEST_TIMEOUT_SEC)
def test_infrastructure_roles(self):
'''
Expand Down
1 change: 1 addition & 0 deletions manifests/configmap.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ data:
external_traffic_policy: "Cluster"
# gcp_credentials: ""
# kubernetes_use_configmaps: "false"
# ignored_annotations: ""
# infrastructure_roles_secret_name: "postgresql-infrastructure-roles"
# infrastructure_roles_secrets: "secretname:monitoring-roles,userkey:user,passwordkey:password,rolekey:inrole"
# inherited_annotations: owned-by
Expand Down
4 changes: 4 additions & 0 deletions manifests/operatorconfiguration.crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,10 @@ spec:
enable_sidecars:
type: boolean
default: true
ignored_annotations:
type: array
items:
type: string
infrastructure_roles_secret_name:
type: string
infrastructure_roles_secrets:
Expand Down
3 changes: 3 additions & 0 deletions manifests/postgresql-operator-default-configuration.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ configuration:
enable_pod_antiaffinity: false
enable_pod_disruption_budget: true
enable_sidecars: true
# ignored_annotations:
# - "deployment-time"
# - "k8s.v1.cni.cncf.io/network-status"
# infrastructure_roles_secret_name: "postgresql-infrastructure-roles"
# infrastructure_roles_secrets:
# - secretname: "monitoring-roles"
Expand Down
8 changes: 8 additions & 0 deletions pkg/apis/acid.zalan.do/v1/crds.go
Original file line number Diff line number Diff line change
Expand Up @@ -1258,6 +1258,14 @@ var OperatorConfigCRDResourceValidation = apiextv1.CustomResourceValidation{
"enable_sidecars": {
Type: "boolean",
},
"ignored_annotations": {
Type: "array",
Items: &apiextv1.JSONSchemaPropsOrArray{
Schema: &apiextv1.JSONSchemaProps{
Type: "string",
},
},
},
"infrastructure_roles_secret_name": {
Type: "string",
},
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/acid.zalan.do/v1/operator_configuration_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ type KubernetesMetaConfiguration struct {
InheritedLabels []string `json:"inherited_labels,omitempty"`
InheritedAnnotations []string `json:"inherited_annotations,omitempty"`
DownscalerAnnotations []string `json:"downscaler_annotations,omitempty"`
IgnoredAnnotations []string `json:"ignored_annotations,omitempty"`
ClusterNameLabel string `json:"cluster_name_label,omitempty"`
DeleteAnnotationDateKey string `json:"delete_annotation_date_key,omitempty"`
DeleteAnnotationNameKey string `json:"delete_annotation_name_key,omitempty"`
Expand Down
23 changes: 20 additions & 3 deletions pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

76 changes: 71 additions & 5 deletions pkg/cluster/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -382,9 +382,10 @@ func (c *Cluster) compareStatefulSetWith(statefulSet *appsv1.StatefulSet) *compa
match = false
reasons = append(reasons, "new statefulset's number of replicas does not match the current one")
}
if !reflect.DeepEqual(c.Statefulset.Annotations, statefulSet.Annotations) {
if changed, reason := c.compareAnnotations(c.Statefulset.Annotations, statefulSet.Annotations); changed {
match = false
needsReplace = true
reasons = append(reasons, "new statefulset's annotations do not match the current one")
reasons = append(reasons, "new statefulset's annotations do not match "+reason)
}

needsRollUpdate, reasons = c.compareContainers("initContainers", c.Statefulset.Spec.Template.Spec.InitContainers, statefulSet.Spec.Template.Spec.InitContainers, needsRollUpdate, reasons)
Expand Down Expand Up @@ -438,10 +439,11 @@ func (c *Cluster) compareStatefulSetWith(statefulSet *appsv1.StatefulSet) *compa
}
}

if !reflect.DeepEqual(c.Statefulset.Spec.Template.Annotations, statefulSet.Spec.Template.Annotations) {
if changed, reason := c.compareAnnotations(c.Statefulset.Spec.Template.Annotations, statefulSet.Spec.Template.Annotations); changed {
match = false
needsReplace = true
needsRollUpdate = true
reasons = append(reasons, "new statefulset's pod template metadata annotations does not match the current one")
reasons = append(reasons, "new statefulset's pod template metadata annotations does not match "+reason)
}
if !reflect.DeepEqual(c.Statefulset.Spec.Template.Spec.SecurityContext, statefulSet.Spec.Template.Spec.SecurityContext) {
needsReplace = true
Expand Down Expand Up @@ -720,6 +722,70 @@ func (c *Cluster) enforceMinResourceLimits(spec *acidv1.PostgresSpec) error {
return nil
}

func (c *Cluster) compareAnnotations(old, new map[string]string) (bool, string) {
ignored := make(map[string]bool)
for _, ignore := range c.OpConfig.IgnoredAnnotations {
ignored[ignore] = true
}

// changed := false
reason := ""

for key := range old {
if _, ok := ignored[key]; ok {
continue
}
if _, ok := new[key]; !ok {
reason += fmt.Sprintf(" Removed '%s'.", key)
}
}

for key := range new {
if _, ok := ignored[key]; ok {
continue
}

v, ok := old[key]
if !ok {
reason += fmt.Sprintf(" Added '%s' with value '%s'.", key, new[key])
} else if v != new[key] {
reason += fmt.Sprintf(" '%s' changed from '%s' to '%s'.", key, v, new[key])
}
}

if reason != "" {
return true, reason
}
return false, ""

}

// SameService compares the Services
func (c *Cluster) compareServices(old, new *v1.Service) (bool, string) {
//TODO: improve comparison
if old.Spec.Type != new.Spec.Type {
return false, fmt.Sprintf("new service's type %q does not match the current one %q",
new.Spec.Type, old.Spec.Type)
}

oldSourceRanges := old.Spec.LoadBalancerSourceRanges
newSourceRanges := new.Spec.LoadBalancerSourceRanges

/* work around Kubernetes 1.6 serializing [] as nil. See https://github.com/kubernetes/kubernetes/issues/43203 */
if (len(oldSourceRanges) != 0) || (len(newSourceRanges) != 0) {
if !util.IsEqualIgnoreOrder(oldSourceRanges, newSourceRanges) {
return false, "new service's LoadBalancerSourceRange does not match the current one"
}
}

if changed, reason := c.compareAnnotations(old.Annotations, new.Annotations); changed {
return !changed, "new service's annotations does not match the current one:" + reason
}

return true, ""

}

// Update changes Kubernetes objects according to the new specification. Unlike the sync case, the missing object
// (i.e. service) is treated as an error
// logical backup cron jobs are an exception: a user-initiated Update can enable a logical backup job
Expand Down Expand Up @@ -821,7 +887,7 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error {
updateFailed = true
return
}
if syncStatefulSet || !reflect.DeepEqual(oldSs, newSs) || !reflect.DeepEqual(oldSpec.Annotations, newSpec.Annotations) {
if syncStatefulSet || !reflect.DeepEqual(oldSs, newSs) {
c.logger.Debugf("syncing statefulsets")
syncStatefulSet = false
// TODO: avoid generating the StatefulSet object twice by passing it to syncStatefulSet
Expand Down
Loading