mirror of
https://github.com/zalando/postgres-operator.git
synced 2026-10-06 22:52:10 +02:00
Configure annotations to be ignored in comparisons during sync (#1823)
* feat: add ignored annotations when comparing during sync Co-authored-by: Felix Kunde <felix-kunde@gmx.de> Co-authored-by: Moshe Immerman <moshe@flanksource.com>
This commit is contained in:
Felix Kunde
co-authored by
Moshe Immerman
Moshe Immerman
parent
36df1bc87c
commit
654d22d04a
+62
-5
@@ -378,9 +378,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)
|
||||
@@ -434,10 +435,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
|
||||
@@ -672,6 +674,61 @@ func comparePorts(a, b []v1.ContainerPort) bool {
|
||||
return true
|
||||
}
|
||||
|
||||
func (c *Cluster) compareAnnotations(old, new map[string]string) (bool, string) {
|
||||
reason := ""
|
||||
ignoredAnnotations := make(map[string]bool)
|
||||
for _, ignore := range c.OpConfig.IgnoredAnnotations {
|
||||
ignoredAnnotations[ignore] = true
|
||||
}
|
||||
|
||||
for key := range old {
|
||||
if _, ok := ignoredAnnotations[key]; ok {
|
||||
continue
|
||||
}
|
||||
if _, ok := new[key]; !ok {
|
||||
reason += fmt.Sprintf(" Removed %q.", key)
|
||||
}
|
||||
}
|
||||
|
||||
for key := range new {
|
||||
if _, ok := ignoredAnnotations[key]; ok {
|
||||
continue
|
||||
}
|
||||
v, ok := old[key]
|
||||
if !ok {
|
||||
reason += fmt.Sprintf(" Added %q with value %q.", key, new[key])
|
||||
} else if v != new[key] {
|
||||
reason += fmt.Sprintf(" %q changed from %q to %q.", key, v, new[key])
|
||||
}
|
||||
}
|
||||
|
||||
return reason != "", reason
|
||||
|
||||
}
|
||||
|
||||
func (c *Cluster) compareServices(old, new *v1.Service) (bool, string) {
|
||||
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
|
||||
@@ -764,7 +821,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
|
||||
|
||||
@@ -3,6 +3,7 @@ package cluster
|
||||
import (
|
||||
"fmt"
|
||||
"reflect"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
@@ -1054,6 +1055,336 @@ func TestCompareEnv(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func newService(ann map[string]string, svcT v1.ServiceType, lbSr []string) *v1.Service {
|
||||
svc := &v1.Service{
|
||||
Spec: v1.ServiceSpec{
|
||||
Type: svcT,
|
||||
LoadBalancerSourceRanges: lbSr,
|
||||
},
|
||||
}
|
||||
svc.Annotations = ann
|
||||
return svc
|
||||
}
|
||||
|
||||
func TestCompareServices(t *testing.T) {
|
||||
testName := "TestCompareServices"
|
||||
cluster := Cluster{
|
||||
Config: Config{
|
||||
OpConfig: config.Config{
|
||||
Resources: config.Resources{
|
||||
IgnoredAnnotations: []string{
|
||||
"k8s.v1.cni.cncf.io/network-status",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
tests := []struct {
|
||||
about string
|
||||
current *v1.Service
|
||||
new *v1.Service
|
||||
reason string
|
||||
match bool
|
||||
}{
|
||||
{
|
||||
about: "two equal services",
|
||||
current: newService(
|
||||
map[string]string{
|
||||
constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do",
|
||||
constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue,
|
||||
},
|
||||
v1.ServiceTypeClusterIP,
|
||||
[]string{"128.141.0.0/16", "137.138.0.0/16"}),
|
||||
new: newService(
|
||||
map[string]string{
|
||||
constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do",
|
||||
constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue,
|
||||
},
|
||||
v1.ServiceTypeClusterIP,
|
||||
[]string{"128.141.0.0/16", "137.138.0.0/16"}),
|
||||
match: true,
|
||||
},
|
||||
{
|
||||
about: "services differ on service type",
|
||||
current: newService(
|
||||
map[string]string{
|
||||
constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do",
|
||||
constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue,
|
||||
},
|
||||
v1.ServiceTypeClusterIP,
|
||||
[]string{"128.141.0.0/16", "137.138.0.0/16"}),
|
||||
new: newService(
|
||||
map[string]string{
|
||||
constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do",
|
||||
constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue,
|
||||
},
|
||||
v1.ServiceTypeLoadBalancer,
|
||||
[]string{"128.141.0.0/16", "137.138.0.0/16"}),
|
||||
match: false,
|
||||
reason: `new service's type "LoadBalancer" does not match the current one "ClusterIP"`,
|
||||
},
|
||||
{
|
||||
about: "services differ on lb source ranges",
|
||||
current: newService(
|
||||
map[string]string{
|
||||
constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do",
|
||||
constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue,
|
||||
},
|
||||
v1.ServiceTypeLoadBalancer,
|
||||
[]string{"128.141.0.0/16", "137.138.0.0/16"}),
|
||||
new: newService(
|
||||
map[string]string{
|
||||
constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do",
|
||||
constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue,
|
||||
},
|
||||
v1.ServiceTypeLoadBalancer,
|
||||
[]string{"185.249.56.0/22"}),
|
||||
match: false,
|
||||
reason: `new service's LoadBalancerSourceRange does not match the current one`,
|
||||
},
|
||||
{
|
||||
about: "new service doesn't have lb source ranges",
|
||||
current: newService(
|
||||
map[string]string{
|
||||
constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do",
|
||||
constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue,
|
||||
},
|
||||
v1.ServiceTypeLoadBalancer,
|
||||
[]string{"128.141.0.0/16", "137.138.0.0/16"}),
|
||||
new: newService(
|
||||
map[string]string{
|
||||
constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do",
|
||||
constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue,
|
||||
},
|
||||
v1.ServiceTypeLoadBalancer,
|
||||
[]string{}),
|
||||
match: false,
|
||||
reason: `new service's LoadBalancerSourceRange does not match the current one`,
|
||||
},
|
||||
{
|
||||
about: "services differ on DNS annotation",
|
||||
current: newService(
|
||||
map[string]string{
|
||||
constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do",
|
||||
constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue,
|
||||
},
|
||||
v1.ServiceTypeLoadBalancer,
|
||||
[]string{"128.141.0.0/16", "137.138.0.0/16"}),
|
||||
new: newService(
|
||||
map[string]string{
|
||||
constants.ZalandoDNSNameAnnotation: "new_clstr.acid.zalan.do",
|
||||
constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue,
|
||||
},
|
||||
v1.ServiceTypeLoadBalancer,
|
||||
[]string{"128.141.0.0/16", "137.138.0.0/16"}),
|
||||
match: false,
|
||||
reason: `new service's annotations does not match the current one: "external-dns.alpha.kubernetes.io/hostname" changed from "clstr.acid.zalan.do" to "new_clstr.acid.zalan.do".`,
|
||||
},
|
||||
{
|
||||
about: "services differ on AWS ELB annotation",
|
||||
current: newService(
|
||||
map[string]string{
|
||||
constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do",
|
||||
constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue,
|
||||
},
|
||||
v1.ServiceTypeLoadBalancer,
|
||||
[]string{"128.141.0.0/16", "137.138.0.0/16"}),
|
||||
new: newService(
|
||||
map[string]string{
|
||||
constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do",
|
||||
constants.ElbTimeoutAnnotationName: "1800",
|
||||
},
|
||||
v1.ServiceTypeLoadBalancer,
|
||||
[]string{"128.141.0.0/16", "137.138.0.0/16"}),
|
||||
match: false,
|
||||
reason: `new service's annotations does not match the current one: "service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout" changed from "3600" to "1800".`,
|
||||
},
|
||||
{
|
||||
about: "service changes existing annotation",
|
||||
current: newService(
|
||||
map[string]string{
|
||||
constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do",
|
||||
constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue,
|
||||
"foo": "bar",
|
||||
},
|
||||
v1.ServiceTypeLoadBalancer,
|
||||
[]string{"128.141.0.0/16", "137.138.0.0/16"}),
|
||||
new: newService(
|
||||
map[string]string{
|
||||
constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do",
|
||||
constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue,
|
||||
"foo": "baz",
|
||||
},
|
||||
v1.ServiceTypeLoadBalancer,
|
||||
[]string{"128.141.0.0/16", "137.138.0.0/16"}),
|
||||
match: false,
|
||||
reason: `new service's annotations does not match the current one: "foo" changed from "bar" to "baz".`,
|
||||
},
|
||||
{
|
||||
about: "service changes multiple existing annotations",
|
||||
current: newService(
|
||||
map[string]string{
|
||||
constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do",
|
||||
constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue,
|
||||
"foo": "bar",
|
||||
"bar": "foo",
|
||||
},
|
||||
v1.ServiceTypeLoadBalancer,
|
||||
[]string{"128.141.0.0/16", "137.138.0.0/16"}),
|
||||
new: newService(
|
||||
map[string]string{
|
||||
constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do",
|
||||
constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue,
|
||||
"foo": "baz",
|
||||
"bar": "fooz",
|
||||
},
|
||||
v1.ServiceTypeLoadBalancer,
|
||||
[]string{"128.141.0.0/16", "137.138.0.0/16"}),
|
||||
match: false,
|
||||
// Test just the prefix to avoid flakiness and map sorting
|
||||
reason: `new service's annotations does not match the current one:`,
|
||||
},
|
||||
{
|
||||
about: "service adds a new custom annotation",
|
||||
current: newService(
|
||||
map[string]string{
|
||||
constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do",
|
||||
constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue,
|
||||
},
|
||||
v1.ServiceTypeLoadBalancer,
|
||||
[]string{"128.141.0.0/16", "137.138.0.0/16"}),
|
||||
new: newService(
|
||||
map[string]string{
|
||||
constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do",
|
||||
constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue,
|
||||
"foo": "bar",
|
||||
},
|
||||
v1.ServiceTypeLoadBalancer,
|
||||
[]string{"128.141.0.0/16", "137.138.0.0/16"}),
|
||||
match: false,
|
||||
reason: `new service's annotations does not match the current one: Added "foo" with value "bar".`,
|
||||
},
|
||||
{
|
||||
about: "service removes a custom annotation",
|
||||
current: newService(
|
||||
map[string]string{
|
||||
constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do",
|
||||
constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue,
|
||||
"foo": "bar",
|
||||
},
|
||||
v1.ServiceTypeLoadBalancer,
|
||||
[]string{"128.141.0.0/16", "137.138.0.0/16"}),
|
||||
new: newService(
|
||||
map[string]string{
|
||||
constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do",
|
||||
constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue,
|
||||
},
|
||||
v1.ServiceTypeLoadBalancer,
|
||||
[]string{"128.141.0.0/16", "137.138.0.0/16"}),
|
||||
match: false,
|
||||
reason: `new service's annotations does not match the current one: Removed "foo".`,
|
||||
},
|
||||
{
|
||||
about: "service removes a custom annotation and adds a new one",
|
||||
current: newService(
|
||||
map[string]string{
|
||||
constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do",
|
||||
constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue,
|
||||
"foo": "bar",
|
||||
},
|
||||
v1.ServiceTypeLoadBalancer,
|
||||
[]string{"128.141.0.0/16", "137.138.0.0/16"}),
|
||||
new: newService(
|
||||
map[string]string{
|
||||
constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do",
|
||||
constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue,
|
||||
"bar": "foo",
|
||||
},
|
||||
v1.ServiceTypeLoadBalancer,
|
||||
[]string{"128.141.0.0/16", "137.138.0.0/16"}),
|
||||
match: false,
|
||||
reason: `new service's annotations does not match the current one: Removed "foo". Added "bar" with value "foo".`,
|
||||
},
|
||||
{
|
||||
about: "service removes a custom annotation, adds a new one and change another",
|
||||
current: newService(
|
||||
map[string]string{
|
||||
constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do",
|
||||
constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue,
|
||||
"foo": "bar",
|
||||
"zalan": "do",
|
||||
},
|
||||
v1.ServiceTypeLoadBalancer,
|
||||
[]string{"128.141.0.0/16", "137.138.0.0/16"}),
|
||||
new: newService(
|
||||
map[string]string{
|
||||
constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do",
|
||||
constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue,
|
||||
"bar": "foo",
|
||||
"zalan": "do.com",
|
||||
},
|
||||
v1.ServiceTypeLoadBalancer,
|
||||
[]string{"128.141.0.0/16", "137.138.0.0/16"}),
|
||||
match: false,
|
||||
// Test just the prefix to avoid flakiness and map sorting
|
||||
reason: `new service's annotations does not match the current one: Removed "foo".`,
|
||||
},
|
||||
{
|
||||
about: "service add annotations",
|
||||
current: newService(
|
||||
map[string]string{},
|
||||
v1.ServiceTypeLoadBalancer,
|
||||
[]string{"128.141.0.0/16", "137.138.0.0/16"}),
|
||||
new: newService(
|
||||
map[string]string{
|
||||
constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do",
|
||||
constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue,
|
||||
},
|
||||
v1.ServiceTypeLoadBalancer,
|
||||
[]string{"128.141.0.0/16", "137.138.0.0/16"}),
|
||||
match: false,
|
||||
// Test just the prefix to avoid flakiness and map sorting
|
||||
reason: `new service's annotations does not match the current one: Added `,
|
||||
},
|
||||
{
|
||||
about: "ignored annotations",
|
||||
current: newService(
|
||||
map[string]string{},
|
||||
v1.ServiceTypeLoadBalancer,
|
||||
[]string{"128.141.0.0/16", "137.138.0.0/16"}),
|
||||
new: newService(
|
||||
map[string]string{
|
||||
"k8s.v1.cni.cncf.io/network-status": "up",
|
||||
},
|
||||
v1.ServiceTypeLoadBalancer,
|
||||
[]string{"128.141.0.0/16", "137.138.0.0/16"}),
|
||||
match: true,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.about, func(t *testing.T) {
|
||||
match, reason := cluster.compareServices(tt.current, tt.new)
|
||||
if match && !tt.match {
|
||||
t.Logf("match=%v current=%v, old=%v reason=%s", match, tt.current.Annotations, tt.new.Annotations, reason)
|
||||
t.Errorf("%s - expected services to do not match: %q and %q", testName, tt.current, tt.new)
|
||||
return
|
||||
}
|
||||
if !match && tt.match {
|
||||
t.Errorf("%s - expected services to be the same: %q and %q", testName, tt.current, tt.new)
|
||||
return
|
||||
}
|
||||
if !match && !tt.match {
|
||||
if !strings.HasPrefix(reason, tt.reason) {
|
||||
t.Errorf("%s - expected reason prefix %s, found %s", testName, tt.reason, reason)
|
||||
return
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestCrossNamespacedSecrets(t *testing.T) {
|
||||
testName := "test secrets in different namespace"
|
||||
clientSet := fake.NewSimpleClientset()
|
||||
|
||||
@@ -912,7 +912,7 @@ func (c *Cluster) syncConnectionPoolerWorker(oldSpec, newSpec *acidv1.Postgresql
|
||||
if service, err = c.KubeClient.Services(c.Namespace).Get(context.TODO(), c.connectionPoolerName(role), metav1.GetOptions{}); err == nil {
|
||||
c.ConnectionPooler[role].Service = service
|
||||
desiredSvc := c.generateConnectionPoolerService(c.ConnectionPooler[role])
|
||||
if match, reason := k8sutil.SameService(service, desiredSvc); !match {
|
||||
if match, reason := c.compareServices(service, desiredSvc); !match {
|
||||
syncReason = append(syncReason, reason)
|
||||
c.logServiceChanges(role, service, desiredSvc, false, reason)
|
||||
newService, err = c.updateService(role, service, desiredSvc)
|
||||
|
||||
+1
-1
@@ -167,7 +167,7 @@ func (c *Cluster) syncService(role PostgresRole) error {
|
||||
if svc, err = c.KubeClient.Services(c.Namespace).Get(context.TODO(), c.serviceName(role), metav1.GetOptions{}); err == nil {
|
||||
c.Services[role] = svc
|
||||
desiredSvc := c.generateService(role, &c.Spec)
|
||||
if match, reason := k8sutil.SameService(svc, desiredSvc); !match {
|
||||
if match, reason := c.compareServices(svc, desiredSvc); !match {
|
||||
c.logServiceChanges(role, svc, desiredSvc, false, reason)
|
||||
updatedSvc, err := c.updateService(role, svc, desiredSvc)
|
||||
if err != nil {
|
||||
|
||||
Reference in New Issue
Block a user