From 24bb45cf62b9e95d55aa513abf5677c008663c80 Mon Sep 17 00:00:00 2001 From: Moshe Immerman Date: Wed, 5 May 2021 22:24:15 +0200 Subject: [PATCH 01/10] feat: add ignored annotations --- cmd/main.go | 5 +- manifests/operatorconfiguration.crd.yaml | 1068 +++++++++-------- .../v1/operator_configuration_type.go | 1 + pkg/cluster/cluster.go | 11 +- .../comparison_test.go} | 29 +- pkg/cluster/sync.go | 2 +- pkg/cluster/util.go | 64 + pkg/controller/controller.go | 3 + pkg/controller/operator_config.go | 1 + pkg/spec/types.go | 1 + pkg/util/config/config.go | 1 + pkg/util/k8sutil/k8sutil.go | 51 - 12 files changed, 645 insertions(+), 592 deletions(-) rename pkg/{util/k8sutil/k8sutil_test.go => cluster/comparison_test.go} (93%) diff --git a/cmd/main.go b/cmd/main.go index 376df0bad..b943314e2 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -2,13 +2,15 @@ package main import ( "flag" - log "github.com/sirupsen/logrus" "os" "os/signal" + "strings" "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" @@ -36,6 +38,7 @@ func init() { flag.BoolVar(&config.NoTeamsAPI, "noteamsapi", false, "Disable all access to the teams API") flag.Parse() + config.IgnoredAnnotations = strings.Split(os.Getenv("IGNORED_ANNOTATIONS"), ",") config.EnableJsonLogging = os.Getenv("ENABLE_JSON_LOGGING") == "true" configMapRawName := os.Getenv("CONFIG_MAP_NAME") diff --git a/manifests/operatorconfiguration.crd.yaml b/manifests/operatorconfiguration.crd.yaml index 47244e5d7..46593ab57 100644 --- a/manifests/operatorconfiguration.crd.yaml +++ b/manifests/operatorconfiguration.crd.yaml @@ -10,549 +10,553 @@ spec: plural: operatorconfigurations singular: operatorconfiguration shortNames: - - opconfig + - opconfig categories: - - all + - all scope: Namespaced versions: - - name: v1 - served: true - storage: true - subresources: - status: {} - additionalPrinterColumns: - - name: Image - type: string - description: Spilo image to be used for Pods - jsonPath: .configuration.docker_image - - name: Cluster-Label - type: string - description: Label for K8s resources created by operator - jsonPath: .configuration.kubernetes.cluster_name_label - - name: Service-Account - type: string - description: Name of service account to be used - jsonPath: .configuration.kubernetes.pod_service_account_name - - name: Min-Instances - type: integer - description: Minimum number of instances per Postgres cluster - jsonPath: .configuration.min_instances - - name: Age - type: date - jsonPath: .metadata.creationTimestamp - schema: - openAPIV3Schema: - type: object - required: - - kind - - apiVersion - - configuration - properties: - kind: - type: string - enum: - - OperatorConfiguration - apiVersion: - type: string - enum: - - acid.zalan.do/v1 - configuration: - type: object - properties: - docker_image: - type: string - default: "registry.opensource.zalan.do/acid/spilo-13:2.0-p6" - enable_crd_validation: - type: boolean - default: true - enable_lazy_spilo_upgrade: - type: boolean - default: false - enable_pgversion_env_var: - type: boolean - default: true - enable_shm_volume: - type: boolean - default: true - enable_spilo_wal_path_compat: - type: boolean - default: false - etcd_host: - type: string - default: "" - kubernetes_use_configmaps: - type: boolean - default: false - max_instances: - type: integer - minimum: -1 # -1 = disabled - default: -1 - min_instances: - type: integer - minimum: -1 # -1 = disabled - default: -1 - resync_period: - type: string - default: "30m" - repair_period: - type: string - default: "5m" - set_memory_request_to_limit: - type: boolean - default: false - sidecar_docker_images: - type: object - additionalProperties: + - name: v1 + served: true + storage: true + subresources: + status: {} + additionalPrinterColumns: + - name: Image + type: string + description: Spilo image to be used for Pods + jsonPath: .configuration.docker_image + - name: Cluster-Label + type: string + description: Label for K8s resources created by operator + jsonPath: .configuration.kubernetes.cluster_name_label + - name: Service-Account + type: string + description: Name of service account to be used + jsonPath: .configuration.kubernetes.pod_service_account_name + - name: Min-Instances + type: integer + description: Minimum number of instances per Postgres cluster + jsonPath: .configuration.min_instances + - name: Age + type: date + jsonPath: .metadata.creationTimestamp + schema: + openAPIV3Schema: + type: object + required: + - kind + - apiVersion + - configuration + properties: + kind: + type: string + enum: + - OperatorConfiguration + apiVersion: + type: string + enum: + - acid.zalan.do/v1 + configuration: + type: object + properties: + docker_image: type: string - sidecars: - type: array - nullable: true - items: + default: "registry.opensource.zalan.do/acid/spilo-13:2.0-p6" + enable_crd_validation: + type: boolean + default: true + enable_lazy_spilo_upgrade: + type: boolean + default: false + enable_pgversion_env_var: + type: boolean + default: true + enable_shm_volume: + type: boolean + default: true + enable_spilo_wal_path_compat: + type: boolean + default: false + etcd_host: + type: string + default: "" + kubernetes_use_configmaps: + type: boolean + default: false + max_instances: + type: integer + minimum: -1 # -1 = disabled + default: -1 + min_instances: + type: integer + minimum: -1 # -1 = disabled + default: -1 + resync_period: + type: string + default: "30m" + repair_period: + type: string + default: "5m" + set_memory_request_to_limit: + type: boolean + default: false + sidecar_docker_images: type: object - x-kubernetes-preserve-unknown-fields: true - workers: - type: integer - minimum: 1 - default: 8 - users: - type: object - properties: - replication_username: - type: string - default: standby - super_username: - type: string - default: postgres - major_version_upgrade: - type: object - properties: - major_version_upgrade_mode: - type: string - default: "off" - minimal_major_version: - type: string - default: "9.5" - target_major_version: - type: string - default: "13" - kubernetes: - type: object - properties: - additional_pod_capabilities: - type: array - items: - type: string - cluster_domain: + additionalProperties: type: string - default: "cluster.local" - cluster_labels: + sidecars: + type: array + nullable: true + items: type: object - additionalProperties: + x-kubernetes-preserve-unknown-fields: true + workers: + type: integer + minimum: 1 + default: 8 + users: + type: object + properties: + replication_username: type: string - default: - application: spilo - cluster_name_label: - type: string - default: "cluster-name" - custom_pod_annotations: - type: object - additionalProperties: + default: standby + super_username: type: string - delete_annotation_date_key: - type: string - delete_annotation_name_key: - type: string - downscaler_annotations: - type: array - items: - type: string - enable_init_containers: - type: boolean - default: true - enable_pod_antiaffinity: - type: boolean - default: false - enable_pod_disruption_budget: - type: boolean - default: true - enable_sidecars: - type: boolean - default: true - infrastructure_roles_secret_name: - type: string - infrastructure_roles_secrets: - type: array - nullable: true - items: + default: postgres + major_version_upgrade: + type: object + properties: + major_version_upgrade_mode: + type: string + default: "off" + minimal_major_version: + type: string + default: "9.5" + target_major_version: + type: string + default: "13" + kubernetes: + type: object + properties: + additional_pod_capabilities: + type: array + items: + type: string + cluster_domain: + type: string + default: "cluster.local" + cluster_labels: type: object - required: - - secretname - - userkey - - passwordkey - properties: - secretname: - type: string - userkey: - type: string - passwordkey: - type: string - rolekey: - type: string - defaultuservalue: - type: string - defaultrolevalue: - type: string - details: - type: string - template: - type: boolean - inherited_annotations: - type: array - items: - type: string - inherited_labels: - type: array - items: - type: string - master_pod_move_timeout: - type: string - default: "20m" - node_readiness_label: - type: object - additionalProperties: + additionalProperties: + type: string + default: + application: spilo + cluster_name_label: type: string - oauth_token_secret_name: - type: string - default: "postgresql-operator" - pdb_name_format: - type: string - default: "postgres-{cluster}-pdb" - pod_antiaffinity_topology_key: - type: string - default: "kubernetes.io/hostname" - pod_environment_configmap: - type: string - pod_environment_secret: - type: string - pod_management_policy: - type: string - enum: - - "ordered_ready" - - "parallel" - default: "ordered_ready" - pod_priority_class_name: - type: string - pod_role_label: - type: string - default: "spilo-role" - pod_service_account_definition: - type: string - default: "" - pod_service_account_name: - type: string - default: "postgres-pod" - pod_service_account_role_binding_definition: - type: string - default: "" - pod_terminate_grace_period: - type: string - default: "5m" - secret_name_template: - type: string - default: "{username}.{cluster}.credentials.{tprkind}.{tprgroup}" - spilo_allow_privilege_escalation: - type: boolean - default: true - spilo_runasuser: - type: integer - spilo_runasgroup: - type: integer - spilo_fsgroup: - type: integer - spilo_privileged: - type: boolean - default: false - storage_resize_mode: - type: string - enum: - - "ebs" - - "pvc" - - "off" - default: "pvc" - toleration: - type: object - additionalProperties: + default: "cluster-name" + ignored_annotations: + type: array + items: + type: string + custom_pod_annotations: + type: object + additionalProperties: + type: string + delete_annotation_date_key: type: string - watched_namespace: - type: string - postgres_pod_resources: - type: object - properties: - default_cpu_limit: - type: string - pattern: '^(\d+m|\d+(\.\d{1,3})?)$' - default: "1" - default_cpu_request: - type: string - pattern: '^(\d+m|\d+(\.\d{1,3})?)$' - default: "100m" - default_memory_limit: - type: string - pattern: '^(\d+(e\d+)?|\d+(\.\d+)?(e\d+)?[EPTGMK]i?)$' - default: "500Mi" - default_memory_request: - type: string - pattern: '^(\d+(e\d+)?|\d+(\.\d+)?(e\d+)?[EPTGMK]i?)$' - default: "100Mi" - min_cpu_limit: - type: string - pattern: '^(\d+m|\d+(\.\d{1,3})?)$' - default: "250m" - min_memory_limit: - type: string - pattern: '^(\d+(e\d+)?|\d+(\.\d+)?(e\d+)?[EPTGMK]i?)$' - default: "250Mi" - timeouts: - type: object - properties: - pod_label_wait_timeout: - type: string - default: "10m" - pod_deletion_wait_timeout: - type: string - default: "10m" - ready_wait_interval: - type: string - default: "4s" - ready_wait_timeout: - type: string - default: "30s" - resource_check_interval: - type: string - default: "3s" - resource_check_timeout: - type: string - default: "10m" - load_balancer: - type: object - properties: - custom_service_annotations: - type: object - additionalProperties: + delete_annotation_name_key: type: string - db_hosted_zone: - type: string - default: "db.example.com" - enable_master_load_balancer: - type: boolean - default: true - enable_replica_load_balancer: - type: boolean - default: false - external_traffic_policy: - type: string - enum: - - "Cluster" - - "Local" - default: "Cluster" - master_dns_name_format: - type: string - default: "{cluster}.{team}.{hostedzone}" - replica_dns_name_format: - type: string - default: "{cluster}-repl.{team}.{hostedzone}" - aws_or_gcp: - type: object - properties: - additional_secret_mount: - type: string - additional_secret_mount_path: - type: string - default: "/meta/credentials" - aws_region: - type: string - default: "eu-central-1" - enable_ebs_gp3_migration: - type: boolean - default: false - enable_ebs_gp3_migration_max_size: - type: integer - default: 1000 - gcp_credentials: - type: string - kube_iam_role: - type: string - log_s3_bucket: - type: string - wal_gs_bucket: - type: string - wal_s3_bucket: - type: string - logical_backup: - type: object - properties: - logical_backup_docker_image: - type: string - default: "registry.opensource.zalan.do/acid/logical-backup:v1.6.2" - logical_backup_google_application_credentials: - type: string - logical_backup_job_prefix: - type: string - default: "logical-backup-" - logical_backup_provider: - type: string - default: "s3" - logical_backup_s3_access_key_id: - type: string - logical_backup_s3_bucket: - type: string - logical_backup_s3_endpoint: - type: string - logical_backup_s3_region: - type: string - logical_backup_s3_secret_access_key: - type: string - logical_backup_s3_sse: - type: string - logical_backup_schedule: - type: string - pattern: '^(\d+|\*)(/\d+)?(\s+(\d+|\*)(/\d+)?){4}$' - default: "30 00 * * *" - debug: - type: object - properties: - debug_logging: - type: boolean - default: true - enable_database_access: - type: boolean - default: true - teams_api: - type: object - properties: - enable_admin_role_for_users: - type: boolean - default: true - enable_postgres_team_crd: - type: boolean - default: true - enable_postgres_team_crd_superusers: - type: boolean - default: false - enable_team_superuser: - type: boolean - default: false - enable_teams_api: - type: boolean - default: true - pam_configuration: - type: string - default: "https://info.example.com/oauth2/tokeninfo?access_token= uid realm=/employees" - pam_role_name: - type: string - default: "zalandos" - postgres_superuser_teams: - type: array - items: - type: string - protected_role_names: - type: array - items: - type: string - default: - - admin - team_admin_role: - type: string - default: "admin" - team_api_role_configuration: - type: object - additionalProperties: + downscaler_annotations: + type: array + items: + type: string + enable_init_containers: + type: boolean + default: true + enable_pod_antiaffinity: + type: boolean + default: false + enable_pod_disruption_budget: + type: boolean + default: true + enable_sidecars: + type: boolean + default: true + infrastructure_roles_secret_name: type: string - default: - log_statement: all - teams_api_url: - type: string - default: "https://teams.example.com/api/" - logging_rest_api: - type: object - properties: - api_port: - type: integer - default: 8080 - cluster_history_entries: - type: integer - default: 1000 - ring_log_lines: - type: integer - default: 100 - scalyr: # deprecated - type: object - properties: - scalyr_api_key: - type: string - scalyr_cpu_limit: - type: string - pattern: '^(\d+m|\d+(\.\d{1,3})?)$' - default: "1" - scalyr_cpu_request: - type: string - pattern: '^(\d+m|\d+(\.\d{1,3})?)$' - default: "100m" - scalyr_image: - type: string - scalyr_memory_limit: - type: string - pattern: '^(\d+(e\d+)?|\d+(\.\d+)?(e\d+)?[EPTGMK]i?)$' - default: "500Mi" - scalyr_memory_request: - type: string - pattern: '^(\d+(e\d+)?|\d+(\.\d+)?(e\d+)?[EPTGMK]i?)$' - default: "50Mi" - scalyr_server_url: - type: string - default: "https://upload.eu.scalyr.com" - connection_pooler: - type: object - properties: - connection_pooler_schema: - type: string - default: "pooler" - connection_pooler_user: - type: string - default: "pooler" - connection_pooler_image: - type: string - default: "registry.opensource.zalan.do/acid/pgbouncer:master-16" - connection_pooler_max_db_connections: - type: integer - default: 60 - connection_pooler_mode: - type: string - enum: - - "session" - - "transaction" - default: "transaction" - connection_pooler_number_of_instances: - type: integer - minimum: 1 - default: 2 - connection_pooler_default_cpu_limit: - type: string - pattern: '^(\d+m|\d+(\.\d{1,3})?)$' - default: "1" - connection_pooler_default_cpu_request: - type: string - pattern: '^(\d+m|\d+(\.\d{1,3})?)$' - default: "500m" - connection_pooler_default_memory_limit: - type: string - pattern: '^(\d+(e\d+)?|\d+(\.\d+)?(e\d+)?[EPTGMK]i?)$' - default: "100Mi" - connection_pooler_default_memory_request: - type: string - pattern: '^(\d+(e\d+)?|\d+(\.\d+)?(e\d+)?[EPTGMK]i?)$' - default: "100Mi" - status: - type: object - additionalProperties: - type: string + infrastructure_roles_secrets: + type: array + nullable: true + items: + type: object + required: + - secretname + - userkey + - passwordkey + properties: + secretname: + type: string + userkey: + type: string + passwordkey: + type: string + rolekey: + type: string + defaultuservalue: + type: string + defaultrolevalue: + type: string + details: + type: string + template: + type: boolean + inherited_annotations: + type: array + items: + type: string + inherited_labels: + type: array + items: + type: string + master_pod_move_timeout: + type: string + default: "20m" + node_readiness_label: + type: object + additionalProperties: + type: string + oauth_token_secret_name: + type: string + default: "postgresql-operator" + pdb_name_format: + type: string + default: "postgres-{cluster}-pdb" + pod_antiaffinity_topology_key: + type: string + default: "kubernetes.io/hostname" + pod_environment_configmap: + type: string + pod_environment_secret: + type: string + pod_management_policy: + type: string + enum: + - "ordered_ready" + - "parallel" + default: "ordered_ready" + pod_priority_class_name: + type: string + pod_role_label: + type: string + default: "spilo-role" + pod_service_account_definition: + type: string + default: "" + pod_service_account_name: + type: string + default: "postgres-pod" + pod_service_account_role_binding_definition: + type: string + default: "" + pod_terminate_grace_period: + type: string + default: "5m" + secret_name_template: + type: string + default: "{username}.{cluster}.credentials.{tprkind}.{tprgroup}" + spilo_allow_privilege_escalation: + type: boolean + default: true + spilo_runasuser: + type: integer + spilo_runasgroup: + type: integer + spilo_fsgroup: + type: integer + spilo_privileged: + type: boolean + default: false + storage_resize_mode: + type: string + enum: + - "ebs" + - "pvc" + - "off" + default: "pvc" + toleration: + type: object + additionalProperties: + type: string + watched_namespace: + type: string + postgres_pod_resources: + type: object + properties: + default_cpu_limit: + type: string + pattern: '^(\d+m|\d+(\.\d{1,3})?)$' + default: "1" + default_cpu_request: + type: string + pattern: '^(\d+m|\d+(\.\d{1,3})?)$' + default: "100m" + default_memory_limit: + type: string + pattern: '^(\d+(e\d+)?|\d+(\.\d+)?(e\d+)?[EPTGMK]i?)$' + default: "500Mi" + default_memory_request: + type: string + pattern: '^(\d+(e\d+)?|\d+(\.\d+)?(e\d+)?[EPTGMK]i?)$' + default: "100Mi" + min_cpu_limit: + type: string + pattern: '^(\d+m|\d+(\.\d{1,3})?)$' + default: "250m" + min_memory_limit: + type: string + pattern: '^(\d+(e\d+)?|\d+(\.\d+)?(e\d+)?[EPTGMK]i?)$' + default: "250Mi" + timeouts: + type: object + properties: + pod_label_wait_timeout: + type: string + default: "10m" + pod_deletion_wait_timeout: + type: string + default: "10m" + ready_wait_interval: + type: string + default: "4s" + ready_wait_timeout: + type: string + default: "30s" + resource_check_interval: + type: string + default: "3s" + resource_check_timeout: + type: string + default: "10m" + load_balancer: + type: object + properties: + custom_service_annotations: + type: object + additionalProperties: + type: string + db_hosted_zone: + type: string + default: "db.example.com" + enable_master_load_balancer: + type: boolean + default: true + enable_replica_load_balancer: + type: boolean + default: false + external_traffic_policy: + type: string + enum: + - "Cluster" + - "Local" + default: "Cluster" + master_dns_name_format: + type: string + default: "{cluster}.{team}.{hostedzone}" + replica_dns_name_format: + type: string + default: "{cluster}-repl.{team}.{hostedzone}" + aws_or_gcp: + type: object + properties: + additional_secret_mount: + type: string + additional_secret_mount_path: + type: string + default: "/meta/credentials" + aws_region: + type: string + default: "eu-central-1" + enable_ebs_gp3_migration: + type: boolean + default: false + enable_ebs_gp3_migration_max_size: + type: integer + default: 1000 + gcp_credentials: + type: string + kube_iam_role: + type: string + log_s3_bucket: + type: string + wal_gs_bucket: + type: string + wal_s3_bucket: + type: string + logical_backup: + type: object + properties: + logical_backup_docker_image: + type: string + default: "registry.opensource.zalan.do/acid/logical-backup:v1.6.2" + logical_backup_google_application_credentials: + type: string + logical_backup_job_prefix: + type: string + default: "logical-backup-" + logical_backup_provider: + type: string + default: "s3" + logical_backup_s3_access_key_id: + type: string + logical_backup_s3_bucket: + type: string + logical_backup_s3_endpoint: + type: string + logical_backup_s3_region: + type: string + logical_backup_s3_secret_access_key: + type: string + logical_backup_s3_sse: + type: string + logical_backup_schedule: + type: string + pattern: '^(\d+|\*)(/\d+)?(\s+(\d+|\*)(/\d+)?){4}$' + default: "30 00 * * *" + debug: + type: object + properties: + debug_logging: + type: boolean + default: true + enable_database_access: + type: boolean + default: true + teams_api: + type: object + properties: + enable_admin_role_for_users: + type: boolean + default: true + enable_postgres_team_crd: + type: boolean + default: true + enable_postgres_team_crd_superusers: + type: boolean + default: false + enable_team_superuser: + type: boolean + default: false + enable_teams_api: + type: boolean + default: true + pam_configuration: + type: string + default: "https://info.example.com/oauth2/tokeninfo?access_token= uid realm=/employees" + pam_role_name: + type: string + default: "zalandos" + postgres_superuser_teams: + type: array + items: + type: string + protected_role_names: + type: array + items: + type: string + default: + - admin + team_admin_role: + type: string + default: "admin" + team_api_role_configuration: + type: object + additionalProperties: + type: string + default: + log_statement: all + teams_api_url: + type: string + default: "https://teams.example.com/api/" + logging_rest_api: + type: object + properties: + api_port: + type: integer + default: 8080 + cluster_history_entries: + type: integer + default: 1000 + ring_log_lines: + type: integer + default: 100 + scalyr: # deprecated + type: object + properties: + scalyr_api_key: + type: string + scalyr_cpu_limit: + type: string + pattern: '^(\d+m|\d+(\.\d{1,3})?)$' + default: "1" + scalyr_cpu_request: + type: string + pattern: '^(\d+m|\d+(\.\d{1,3})?)$' + default: "100m" + scalyr_image: + type: string + scalyr_memory_limit: + type: string + pattern: '^(\d+(e\d+)?|\d+(\.\d+)?(e\d+)?[EPTGMK]i?)$' + default: "500Mi" + scalyr_memory_request: + type: string + pattern: '^(\d+(e\d+)?|\d+(\.\d+)?(e\d+)?[EPTGMK]i?)$' + default: "50Mi" + scalyr_server_url: + type: string + default: "https://upload.eu.scalyr.com" + connection_pooler: + type: object + properties: + connection_pooler_schema: + type: string + default: "pooler" + connection_pooler_user: + type: string + default: "pooler" + connection_pooler_image: + type: string + default: "registry.opensource.zalan.do/acid/pgbouncer:master-16" + connection_pooler_max_db_connections: + type: integer + default: 60 + connection_pooler_mode: + type: string + enum: + - "session" + - "transaction" + default: "transaction" + connection_pooler_number_of_instances: + type: integer + minimum: 1 + default: 2 + connection_pooler_default_cpu_limit: + type: string + pattern: '^(\d+m|\d+(\.\d{1,3})?)$' + default: "1" + connection_pooler_default_cpu_request: + type: string + pattern: '^(\d+m|\d+(\.\d{1,3})?)$' + default: "500m" + connection_pooler_default_memory_limit: + type: string + pattern: '^(\d+(e\d+)?|\d+(\.\d+)?(e\d+)?[EPTGMK]i?)$' + default: "100Mi" + connection_pooler_default_memory_request: + type: string + pattern: '^(\d+(e\d+)?|\d+(\.\d+)?(e\d+)?[EPTGMK]i?)$' + default: "100Mi" + status: + type: object + additionalProperties: + type: string diff --git a/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go b/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go index 78b618b78..7bebd704d 100644 --- a/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go +++ b/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go @@ -74,6 +74,7 @@ type KubernetesMetaConfiguration struct { InfrastructureRolesDefs []*config.InfrastructureRole `json:"infrastructure_roles_secrets,omitempty"` PodRoleLabel string `json:"pod_role_label,omitempty"` ClusterLabels map[string]string `json:"cluster_labels,omitempty"` + IgnoredAnnotations []string `json:"ignored_annotations,omitempty"` InheritedLabels []string `json:"inherited_labels,omitempty"` InheritedAnnotations []string `json:"inherited_annotations,omitempty"` DownscalerAnnotations []string `json:"downscaler_annotations,omitempty"` diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 512f6a6c9..c44ff8750 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -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 diff --git a/pkg/util/k8sutil/k8sutil_test.go b/pkg/cluster/comparison_test.go similarity index 93% rename from pkg/util/k8sutil/k8sutil_test.go rename to pkg/cluster/comparison_test.go index b3e768501..e57049199 100644 --- a/pkg/util/k8sutil/k8sutil_test.go +++ b/pkg/cluster/comparison_test.go @@ -1,9 +1,10 @@ -package k8sutil +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" @@ -21,6 +22,15 @@ func newsService(ann map[string]string, svcT v1.ServiceType, lbSr []string) *v1. } 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 @@ -288,11 +298,26 @@ func TestSameService(t *testing.T) { // 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 := SameService(tt.current, tt.new) + 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 } diff --git a/pkg/cluster/sync.go b/pkg/cluster/sync.go index 3f08cfb4d..f3f2d10e9 100644 --- a/pkg/cluster/sync.go +++ b/pkg/cluster/sync.go @@ -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) diff --git a/pkg/cluster/util.go b/pkg/cluster/util.go index fa8a52a1b..9ec6f4e8d 100644 --- a/pkg/cluster/util.go +++ b/pkg/cluster/util.go @@ -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)) diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index f992ff782..4a3ba0ec8 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -184,6 +184,9 @@ func (c *Controller) modifyConfigFromEnvironment() { if c.config.NoTeamsAPI { c.opConfig.EnableTeamsAPI = false } + + c.opConfig.IgnoredAnnotations = append(c.opConfig.IgnoredAnnotations, c.config.IgnoredAnnotations...) + scalyrAPIKey := os.Getenv("SCALYR_API_KEY") if scalyrAPIKey != "" { c.opConfig.ScalyrAPIKey = scalyrAPIKey diff --git a/pkg/controller/operator_config.go b/pkg/controller/operator_config.go index 3c0302cab..6c51d6bcc 100644 --- a/pkg/controller/operator_config.go +++ b/pkg/controller/operator_config.go @@ -60,6 +60,7 @@ func (c *Controller) importConfigurationFromCRD(fromCRD *acidv1.OperatorConfigur result.TargetMajorVersion = util.Coalesce(fromCRD.MajorVersionUpgrade.TargetMajorVersion, "13") // kubernetes config + result.IgnoredAnnotations = fromCRD.Kubernetes.IgnoredAnnotations result.CustomPodAnnotations = fromCRD.Kubernetes.CustomPodAnnotations result.PodServiceAccountName = util.Coalesce(fromCRD.Kubernetes.PodServiceAccountName, "postgres-pod") result.PodServiceAccountDefinition = fromCRD.Kubernetes.PodServiceAccountDefinition diff --git a/pkg/spec/types.go b/pkg/spec/types.go index 78c79e1b3..8ecffb299 100644 --- a/pkg/spec/types.go +++ b/pkg/spec/types.go @@ -114,6 +114,7 @@ type ControllerConfig struct { CRDReadyWaitTimeout time.Duration ConfigMapName NamespacedName Namespace string + IgnoredAnnotations []string EnableJsonLogging bool } diff --git a/pkg/util/config/config.go b/pkg/util/config/config.go index a5d144051..4a5ef917c 100644 --- a/pkg/util/config/config.go +++ b/pkg/util/config/config.go @@ -181,6 +181,7 @@ type Config struct { EnablePostgresTeamCRDSuperusers bool `name:"enable_postgres_team_crd_superusers" default:"false"` EnableMasterLoadBalancer bool `name:"enable_master_load_balancer" default:"true"` EnableReplicaLoadBalancer bool `name:"enable_replica_load_balancer" default:"false"` + IgnoredAnnotations []string `json:"ignored_annotations"` CustomServiceAnnotations map[string]string `name:"custom_service_annotations"` CustomPodAnnotations map[string]string `name:"custom_pod_annotations"` EnablePodAntiAffinity bool `name:"enable_pod_antiaffinity" default:"false"` diff --git a/pkg/util/k8sutil/k8sutil.go b/pkg/util/k8sutil/k8sutil.go index dd6ec1e8b..7e204cb89 100644 --- a/pkg/util/k8sutil/k8sutil.go +++ b/pkg/util/k8sutil/k8sutil.go @@ -197,57 +197,6 @@ func (client *KubernetesClient) SetPostgresCRDStatus(clusterName spec.Namespaced return pg, nil } -// SameService compares the Services -func SameService(cur, new *v1.Service) (match bool, reason string) { - //TODO: improve comparison - if cur.Spec.Type != new.Spec.Type { - return false, fmt.Sprintf("new service's type %q does not match the current one %q", - new.Spec.Type, cur.Spec.Type) - } - - oldSourceRanges := cur.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" - } - } - - match = true - - reasonPrefix := "new service's annotations does not match the current one:" - for ann := range cur.Annotations { - if _, ok := new.Annotations[ann]; !ok { - match = false - if len(reason) == 0 { - reason = reasonPrefix - } - reason += fmt.Sprintf(" Removed '%s'.", ann) - } - } - - for ann := range new.Annotations { - v, ok := cur.Annotations[ann] - if !ok { - if len(reason) == 0 { - reason = reasonPrefix - } - reason += fmt.Sprintf(" Added '%s' with value '%s'.", ann, new.Annotations[ann]) - match = false - } else if v != new.Annotations[ann] { - if len(reason) == 0 { - reason = reasonPrefix - } - reason += fmt.Sprintf(" '%s' changed from '%s' to '%s'.", ann, v, new.Annotations[ann]) - match = false - } - } - - return match, reason -} - // SamePDB compares the PodDisruptionBudgets func SamePDB(cur, new *policybeta1.PodDisruptionBudget) (match bool, reason string) { //TODO: improve comparison From 3ecd1e94a0083e6f18cb26e8381ffd84440cfc24 Mon Sep 17 00:00:00 2001 From: Moshe Immerman Date: Wed, 12 May 2021 17:11:12 +0200 Subject: [PATCH 02/10] feat: add ignored annotations Co-authored-by: Felix Kunde --- pkg/util/config/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/util/config/config.go b/pkg/util/config/config.go index 4a5ef917c..241e109a2 100644 --- a/pkg/util/config/config.go +++ b/pkg/util/config/config.go @@ -181,7 +181,7 @@ type Config struct { EnablePostgresTeamCRDSuperusers bool `name:"enable_postgres_team_crd_superusers" default:"false"` EnableMasterLoadBalancer bool `name:"enable_master_load_balancer" default:"true"` EnableReplicaLoadBalancer bool `name:"enable_replica_load_balancer" default:"false"` - IgnoredAnnotations []string `json:"ignored_annotations"` + IgnoredAnnotations []string `name:"ignored_annotations"` CustomServiceAnnotations map[string]string `name:"custom_service_annotations"` CustomPodAnnotations map[string]string `name:"custom_pod_annotations"` EnablePodAntiAffinity bool `name:"enable_pod_antiaffinity" default:"false"` From 181d87f9b2201248c8a43d221e1d9f212f012906 Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Wed, 23 Mar 2022 14:30:36 +0100 Subject: [PATCH 03/10] change unit test name --- pkg/cluster/cluster_test.go | 29 +++++++---------------------- 1 file changed, 7 insertions(+), 22 deletions(-) diff --git a/pkg/cluster/cluster_test.go b/pkg/cluster/cluster_test.go index c2b3118e5..ae87fcf0c 100644 --- a/pkg/cluster/cluster_test.go +++ b/pkg/cluster/cluster_test.go @@ -1074,27 +1074,10 @@ func newService(ann map[string]string, svcT v1.ServiceType, lbSr []string) *v1.S return svc } -func TestSameService(t *testing.T) { - testName := "test comparing services" - client, _ := newFakeK8sServiceClient() - namespace := "default" - - pg := acidv1.Postgresql{ - ObjectMeta: metav1.ObjectMeta{ - Name: "acid-fake-cluster", - Namespace: namespace, - Annotations: map[string]string{ - "deployment-time": "2022-02-02 12:00:00", - }, - }, - Spec: acidv1.PostgresSpec{ - Volume: acidv1.Volume{ - Size: "1Gi", - }, - }, - } - var cluster = New( - Config{ +func TestCompareServices(t *testing.T) { + testName := "TestCompareServices" + cluster := Cluster{ + Config: Config{ OpConfig: config.Config{ Resources: config.Resources{ IgnoredAnnotations: []string{ @@ -1102,7 +1085,8 @@ func TestSameService(t *testing.T) { }, }, }, - }, client, pg, logger, eventRecorder) + }, + } tests := []struct { about string @@ -1386,6 +1370,7 @@ func TestSameService(t *testing.T) { match: true, }, } + for _, tt := range tests { t.Run(tt.about, func(t *testing.T) { match, reason := cluster.compareServices(tt.current, tt.new) From b037bc37770772c89c549132ca2d2e917c306bb2 Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Wed, 23 Mar 2022 14:38:42 +0100 Subject: [PATCH 04/10] fix merge errors --- manifests/operatorconfiguration.crd.yaml | 114 +++++++++++------------ pkg/cluster/cluster_test.go | 8 -- 2 files changed, 52 insertions(+), 70 deletions(-) diff --git a/manifests/operatorconfiguration.crd.yaml b/manifests/operatorconfiguration.crd.yaml index 2d22a5daa..173e83c51 100644 --- a/manifests/operatorconfiguration.crd.yaml +++ b/manifests/operatorconfiguration.crd.yaml @@ -10,9 +10,9 @@ spec: plural: operatorconfigurations singular: operatorconfiguration shortNames: - - opconfig + - opconfig categories: - - all + - all scope: Namespaced versions: - name: v1 @@ -115,46 +115,10 @@ spec: type: object additionalProperties: type: string - default: "registry.opensource.zalan.do/acid/spilo-13:2.0-p6" - enable_crd_validation: - type: boolean - default: true - enable_lazy_spilo_upgrade: - type: boolean - default: false - enable_pgversion_env_var: - type: boolean - default: true - enable_shm_volume: - type: boolean - default: true - enable_spilo_wal_path_compat: - type: boolean - default: false - etcd_host: - type: string - default: "" - kubernetes_use_configmaps: - type: boolean - default: false - max_instances: - type: integer - minimum: -1 # -1 = disabled - default: -1 - min_instances: - type: integer - minimum: -1 # -1 = disabled - default: -1 - resync_period: - type: string - default: "30m" - repair_period: - type: string - default: "5m" - set_memory_request_to_limit: - type: boolean - default: false - sidecar_docker_images: + sidecars: + type: array + nullable: true + items: type: object x-kubernetes-preserve-unknown-fields: true workers: @@ -209,23 +173,19 @@ spec: type: string cluster_domain: type: string - sidecars: - type: array - nullable: true - items: + default: "cluster.local" + cluster_labels: type: object - x-kubernetes-preserve-unknown-fields: true - workers: - type: integer - minimum: 1 - default: 8 - users: - type: object - properties: - replication_username: + additionalProperties: type: string - default: standby - super_username: + default: + application: spilo + cluster_name_label: + type: string + default: "cluster-name" + custom_pod_annotations: + type: object + additionalProperties: type: string delete_annotation_date_key: type: string @@ -261,11 +221,41 @@ spec: nullable: true items: type: object - additionalProperties: - type: string - default: - application: spilo - cluster_name_label: + required: + - secretname + - userkey + - passwordkey + properties: + secretname: + type: string + userkey: + type: string + passwordkey: + type: string + rolekey: + type: string + defaultuservalue: + type: string + defaultrolevalue: + type: string + details: + type: string + template: + type: boolean + inherited_annotations: + type: array + items: + type: string + inherited_labels: + type: array + items: + type: string + master_pod_move_timeout: + type: string + default: "20m" + node_readiness_label: + type: object + additionalProperties: type: string node_readiness_label_merge: type: string diff --git a/pkg/cluster/cluster_test.go b/pkg/cluster/cluster_test.go index ae87fcf0c..1a45bdfb8 100644 --- a/pkg/cluster/cluster_test.go +++ b/pkg/cluster/cluster_test.go @@ -1055,14 +1055,6 @@ func TestCompareEnv(t *testing.T) { } } -func newFakeK8sServiceClient() (k8sutil.KubernetesClient, *fake.Clientset) { - clientSet := fake.NewSimpleClientset() - - return k8sutil.KubernetesClient{ - ServicesGetter: clientSet.CoreV1(), - }, clientSet -} - func newService(ann map[string]string, svcT v1.ServiceType, lbSr []string) *v1.Service { svc := &v1.Service{ Spec: v1.ServiceSpec{ From 8c950cfcbfdae3b1fee04452672dee553657c883 Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Wed, 23 Mar 2022 14:46:22 +0100 Subject: [PATCH 05/10] update generated code --- pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go b/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go index 8ac0adaef..26d930f2b 100644 --- a/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go +++ b/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go @@ -213,11 +213,6 @@ func (in *KubernetesMetaConfiguration) DeepCopyInto(out *KubernetesMetaConfigura (*out)[key] = val } } - if in.IgnoredAnnotations != nil { - in, out := &in.IgnoredAnnotations, &out.IgnoredAnnotations - *out = make([]string, len(*in)) - copy(*out, *in) - } if in.InheritedLabels != nil { in, out := &in.InheritedLabels, &out.InheritedLabels *out = make([]string, len(*in)) @@ -233,6 +228,11 @@ func (in *KubernetesMetaConfiguration) DeepCopyInto(out *KubernetesMetaConfigura *out = make([]string, len(*in)) copy(*out, *in) } + if in.IgnoredAnnotations != nil { + in, out := &in.IgnoredAnnotations, &out.IgnoredAnnotations + *out = make([]string, len(*in)) + copy(*out, *in) + } if in.NodeReadinessLabel != nil { in, out := &in.NodeReadinessLabel, &out.NodeReadinessLabel *out = make(map[string]string, len(*in)) From af4e8e71ae7715e2976ae79906ef97e8a18b1b82 Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Wed, 23 Mar 2022 14:55:06 +0100 Subject: [PATCH 06/10] revert unrelated changes --- docs/reference/operator_parameters.md | 6 +++--- e2e/tests/test_e2e.py | 2 +- pkg/controller/controller.go | 1 - 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/docs/reference/operator_parameters.md b/docs/reference/operator_parameters.md index 96c940949..13840231e 100644 --- a/docs/reference/operator_parameters.md +++ b/docs/reference/operator_parameters.md @@ -289,9 +289,9 @@ configuration they are grouped under the `kubernetes` key. * **ignored_annotations** Some K8s tools inject and update annotations out of the Postgres Operator - control. This can cause rolling updates on each sync cycle of clusters. - With this option you can sepecify an array of annotations keys that should - be ignored when comparing K8s resources. The default is empty. + 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 diff --git a/e2e/tests/test_e2e.py b/e2e/tests/test_e2e.py index 9bae875df..1edeb3bb4 100644 --- a/e2e/tests/test_e2e.py +++ b/e2e/tests/test_e2e.py @@ -204,7 +204,7 @@ def test_additional_pod_capabilities(self): } # get node and replica (expected target of new master) - master_nodes, replica_nodes = k8s.get_pg_nodes(cluster_label) + _, replica_nodes = k8s.get_pg_nodes(cluster_label) try: k8s.update_config(patch_capabilities) diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index d62036c67..de0dec69f 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -186,7 +186,6 @@ func (c *Controller) modifyConfigFromEnvironment() { if c.config.NoTeamsAPI { c.opConfig.EnableTeamsAPI = false } - scalyrAPIKey := os.Getenv("SCALYR_API_KEY") if scalyrAPIKey != "" { c.opConfig.ScalyrAPIKey = scalyrAPIKey From 80d4eb3c0db39fa8bbb4fd9a4a17a1a9d68b3a20 Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Thu, 24 Mar 2022 10:35:40 +0100 Subject: [PATCH 07/10] reflect code review --- pkg/cluster/cluster.go | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 3b94c9230..f801aef6d 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -723,16 +723,14 @@ func (c *Cluster) enforceMinResourceLimits(spec *acidv1.PostgresSpec) error { } func (c *Cluster) compareAnnotations(old, new map[string]string) (bool, string) { - ignored := make(map[string]bool) + reason := "" + ignoredAnnotations := make(map[string]bool) for _, ignore := range c.OpConfig.IgnoredAnnotations { - ignored[ignore] = true + ignoredAnnotations[ignore] = true } - // changed := false - reason := "" - for key := range old { - if _, ok := ignored[key]; ok { + if _, ok := ignoredAnnotations[key]; ok { continue } if _, ok := new[key]; !ok { @@ -741,10 +739,9 @@ func (c *Cluster) compareAnnotations(old, new map[string]string) (bool, string) } for key := range new { - if _, ok := ignored[key]; ok { + if _, ok := ignoredAnnotations[key]; ok { continue } - v, ok := old[key] if !ok { reason += fmt.Sprintf(" Added '%s' with value '%s'.", key, new[key]) @@ -753,16 +750,11 @@ func (c *Cluster) compareAnnotations(old, new map[string]string) (bool, string) } } - if reason != "" { - return true, reason - } - return false, "" + return reason != "", reason } -// 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) @@ -783,7 +775,6 @@ func (c *Cluster) compareServices(old, new *v1.Service) (bool, string) { } return true, "" - } // Update changes Kubernetes objects according to the new specification. Unlike the sync case, the missing object From 6a3645aeec05f7cf091e31ccc9ed797fa65ba5d4 Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Thu, 24 Mar 2022 11:24:12 +0100 Subject: [PATCH 08/10] fix e2e test --- charts/postgres-operator/values.yaml | 4 +- e2e/tests/test_e2e.py | 52 +++++++++++-------- ...gresql-operator-default-configuration.yaml | 3 +- pkg/cluster/cluster.go | 8 +-- 4 files changed, 35 insertions(+), 32 deletions(-) diff --git a/charts/postgres-operator/values.yaml b/charts/postgres-operator/values.yaml index 7ad804114..2ce26d18a 100644 --- a/charts/postgres-operator/values.yaml +++ b/charts/postgres-operator/values.yaml @@ -127,9 +127,7 @@ configKubernetes: # annotations to be ignored when comparing statefulsets, services etc. # ignored_annotations: - # - "deployment-time" - # - "k8s.v1.cni.cncf.io/network-status" - + # - 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 diff --git a/e2e/tests/test_e2e.py b/e2e/tests/test_e2e.py index 1edeb3bb4..11beccd6b 100644 --- a/e2e/tests/test_e2e.py +++ b/e2e/tests/test_e2e.py @@ -707,39 +707,45 @@ def test_enable_load_balancer(self): @timeout_decorator.timeout(TEST_TIMEOUT_SEC) def test_ignored_annotations(self): ''' - Test if injected annotation does not cause failover when listed under ignored_annotations + Test if injected annotation does not cause replacement of resources 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 = { + annotation_patch = { "metadata": { "annotations": { - "deployment-time": "2022-04-01 12:00:00" + "k8s-status": "healthy" }, } } - 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") + try: + sts = k8s.api.apps_v1.read_namespaced_stateful_set('acid-minimal-cluster', 'default') + old_sts_creation_timestamp = sts.metadata.creation_timestamp + k8s.api.apps_v1.patch_namespaced_stateful_set(sts.metadata.name, sts.metadata.namespace, annotation_patch) + svc = k8s.api.core_v1.read_namespaced_service('acid-minimal-cluster', 'default') + old_svc_creation_timestamp = svc.metadata.creation_timestamp + k8s.api.core_v1.patch_namespaced_service(svc.metadata.name, svc.metadata.namespace, annotation_patch) + + patch_config_ignored_annotations = { + "data": { + "ignored_annotations": "k8s-status", + } + } + k8s.update_config(patch_config_ignored_annotations) + self.eventuallyEqual(lambda: k8s.get_operator_state(), {"0": "idle"}, "Operator does not get in sync") - current_master_node, _ = k8s.get_pg_nodes(cluster_label) - self.eventuallyEqual(lambda: master_node, current_master_node, "unexpected rolling update happened") + sts = k8s.api.apps_v1.read_namespaced_stateful_set('acid-minimal-cluster', 'default') + new_sts_creation_timestamp = sts.metadata.creation_timestamp + svc = k8s.api.core_v1.read_namespaced_service('acid-minimal-cluster', 'default') + new_svc_creation_timestamp = svc.metadata.creation_timestamp + + self.eventuallyEqual(old_sts_creation_timestamp, new_sts_creation_timestamp, "unexpected replacement of statefulset on sync") + self.eventuallyEqual(old_svc_creation_timestamp, new_svc_creation_timestamp, "unexpected replacement of master service on sync") + + except timeout_decorator.TimeoutError: + print('Operator log: {}'.format(k8s.get_operator_log())) + raise @timeout_decorator.timeout(TEST_TIMEOUT_SEC) def test_infrastructure_roles(self): diff --git a/manifests/postgresql-operator-default-configuration.yaml b/manifests/postgresql-operator-default-configuration.yaml index 1d16e51fd..a5ceb8d2a 100644 --- a/manifests/postgresql-operator-default-configuration.yaml +++ b/manifests/postgresql-operator-default-configuration.yaml @@ -60,8 +60,7 @@ configuration: enable_pod_disruption_budget: true enable_sidecars: true # ignored_annotations: - # - "deployment-time" - # - "k8s.v1.cni.cncf.io/network-status" + # - k8s.v1.cni.cncf.io/network-status # infrastructure_roles_secret_name: "postgresql-infrastructure-roles" # infrastructure_roles_secrets: # - secretname: "monitoring-roles" diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index f801aef6d..3bdbb0fe8 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -385,7 +385,7 @@ func (c *Cluster) compareStatefulSetWith(statefulSet *appsv1.StatefulSet) *compa 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 "+reason) + 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) @@ -734,7 +734,7 @@ func (c *Cluster) compareAnnotations(old, new map[string]string) (bool, string) continue } if _, ok := new[key]; !ok { - reason += fmt.Sprintf(" Removed '%s'.", key) + reason += fmt.Sprintf(" Removed %q.", key) } } @@ -744,9 +744,9 @@ func (c *Cluster) compareAnnotations(old, new map[string]string) (bool, string) } v, ok := old[key] if !ok { - reason += fmt.Sprintf(" Added '%s' with value '%s'.", key, new[key]) + reason += fmt.Sprintf(" Added %q with value %q.", key, new[key]) } else if v != new[key] { - reason += fmt.Sprintf(" '%s' changed from '%s' to '%s'.", key, v, new[key]) + reason += fmt.Sprintf(" %q changed from %q to %q.", key, v, new[key]) } } From 92ef662cd54f308266c25a0091cd30fb36596aa2 Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Thu, 24 Mar 2022 13:17:07 +0100 Subject: [PATCH 09/10] fix quoting in unit test --- pkg/cluster/cluster_test.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/pkg/cluster/cluster_test.go b/pkg/cluster/cluster_test.go index 1a45bdfb8..acfecc166 100644 --- a/pkg/cluster/cluster_test.go +++ b/pkg/cluster/cluster_test.go @@ -1179,7 +1179,7 @@ func TestCompareServices(t *testing.T) { 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'.`, + 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", @@ -1198,7 +1198,7 @@ func TestCompareServices(t *testing.T) { 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'.`, + 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", @@ -1219,7 +1219,7 @@ func TestCompareServices(t *testing.T) { 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'.`, + reason: `new service's annotations does not match the current one: "foo" changed from "bar" to "baz".`, }, { about: "service changes multiple existing annotations", @@ -1263,7 +1263,7 @@ func TestCompareServices(t *testing.T) { 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'.`, + reason: `new service's annotations does not match the current one: Added "foo" with value "bar".`, }, { about: "service removes a custom annotation", @@ -1283,7 +1283,7 @@ func TestCompareServices(t *testing.T) { 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'.`, + reason: `new service's annotations does not match the current one: Removed "foo".`, }, { about: "service removes a custom annotation and adds a new one", @@ -1304,7 +1304,7 @@ func TestCompareServices(t *testing.T) { 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'.`, + 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", @@ -1328,7 +1328,7 @@ func TestCompareServices(t *testing.T) { []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'.`, + reason: `new service's annotations does not match the current one: Removed "foo".`, }, { about: "service add annotations", @@ -1368,16 +1368,16 @@ func TestCompareServices(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) + 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) + 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) + t.Errorf("%s - expected reason prefix %s, found %s", testName, tt.reason, reason) return } } From 9d510482523f44e4e0330ac747aafcdf5f42ab6f Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Thu, 24 Mar 2022 13:42:58 +0100 Subject: [PATCH 10/10] fix e2e test --- e2e/tests/test_e2e.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/e2e/tests/test_e2e.py b/e2e/tests/test_e2e.py index 11beccd6b..b2977b687 100644 --- a/e2e/tests/test_e2e.py +++ b/e2e/tests/test_e2e.py @@ -740,8 +740,8 @@ def test_ignored_annotations(self): svc = k8s.api.core_v1.read_namespaced_service('acid-minimal-cluster', 'default') new_svc_creation_timestamp = svc.metadata.creation_timestamp - self.eventuallyEqual(old_sts_creation_timestamp, new_sts_creation_timestamp, "unexpected replacement of statefulset on sync") - self.eventuallyEqual(old_svc_creation_timestamp, new_svc_creation_timestamp, "unexpected replacement of master service on sync") + self.assertEqual(old_sts_creation_timestamp, new_sts_creation_timestamp, "unexpected replacement of statefulset on sync") + self.assertEqual(old_svc_creation_timestamp, new_svc_creation_timestamp, "unexpected replacement of master service on sync") except timeout_decorator.TimeoutError: print('Operator log: {}'.format(k8s.get_operator_log()))