public_git/postgres-operator
1
0
Fork 0
mirror of https://github.com/zalando/postgres-operator.git synced 2026-10-07 00:31:50 +02:00
Code Issues Projects Releases Wiki Activity

feat: add ignored annotations

Browse Source
This commit is contained in:
Moshe Immerman
2021-05-05 22:24:15 +02:00
parent 32e6c135b9
commit 24bb45cf62
12 changed files with 655 additions and 602 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/cluster/cluster.go
+6 -5
View File
@@ -359,10 +359,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)
@@ -411,11 +411,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) {
match = false
@@ -689,7 +689,8 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error {
updateFailed = true
return
}
if syncStatefulSet || !reflect.DeepEqual(oldSs, newSs) || !reflect.DeepEqual(oldSpec.Annotations, newSpec.Annotations) {
annotationsChanged, _ := c.compareAnnotations(oldSpec.Annotations, newSpec.Annotations)
if syncStatefulSet || !reflect.DeepEqual(oldSs, newSs) || annotationsChanged {
c.logger.Debugf("syncing statefulsets")
syncStatefulSet = false
// TODO: avoid generating the StatefulSet object twice by passing it to syncStatefulSet
pkg/cluster/comparison_test.go
+336
View File
@@ -0,0 +1,336 @@
package cluster
import (
"strings"
"testing"
"github.com/zalando/postgres-operator/pkg/util/config"
"github.com/zalando/postgres-operator/pkg/util/constants"
v1 "k8s.io/api/core/v1"
)
func newsService(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 TestSameService(t *testing.T) {
cluster := Cluster{
Config: Config{
OpConfig: config.Config{
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: newsService(
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: newsService(
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: newsService(
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: newsService(
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: newsService(
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: newsService(
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: newsService(
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: newsService(
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: newsService(
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: newsService(
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: newsService(
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: newsService(
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: newsService(
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: newsService(
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: newsService(
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: newsService(
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: newsService(
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: newsService(
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: newsService(
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: newsService(
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: newsService(
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: newsService(
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: newsService(
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: newsService(
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: newsService(
map[string]string{},
v1.ServiceTypeLoadBalancer,
[]string{"128.141.0.0/16", "137.138.0.0/16"}),
new: newsService(
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: newsService(
map[string]string{},
v1.ServiceTypeLoadBalancer,
[]string{"128.141.0.0/16", "137.138.0.0/16"}),
new: newsService(
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.SameService(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("expected services to do not match: '%q' and '%q'", tt.current, tt.new)
return
}
if !match && tt.match {
t.Errorf("expected services to be the same: '%q' and '%q'", tt.current, tt.new)
return
}
if !match && !tt.match {
if !strings.HasPrefix(reason, tt.reason) {
t.Errorf("expected reason prefix '%s', found '%s'", tt.reason, reason)
return
}
}
})
}
}
pkg/cluster/sync.go
+1 -1
View File
@@ -153,7 +153,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.SameService(svc, desiredSvc); !match {
c.logServiceChanges(role, svc, desiredSvc, false, reason)
if err = c.updateService(role, desiredSvc); err != nil {
return fmt.Errorf("could not update %s service to match desired state: %v", role, err)
pkg/cluster/util.go
+64
View File
@@ -186,6 +186,70 @@ func logNiceDiff(log *logrus.Entry, old, new interface{}) {
}
}
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) SameService(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 !reflect.DeepEqual(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, ""
}
func (c *Cluster) logStatefulSetChanges(old, new *appsv1.StatefulSet, isUpdate bool, reasons []string) {
if isUpdate {
c.logger.Infof("statefulset %s has been changed", util.NameFromMeta(old.ObjectMeta))