From e4c0c427a30114028e862bc31171d01b745c3759 Mon Sep 17 00:00:00 2001
From: ERIK <bo.jiang@daocloud.io>
Date: Fri, 16 May 2025 18:55:14 +0800
Subject: [PATCH] improve NTP package conflict handling (#12212)

Signed-off-by: bo.jiang <bo.jiang@daocloud.io>
---
 roles/kubernetes/preinstall/defaults/main.yml   | 11 -----------
 .../tasks/0081-ntp-configurations.yml           |  8 --------
 roles/kubespray_defaults/defaults/main/main.yml | 17 +++++++++++++++++
 roles/system_packages/tasks/main.yml            | 11 ++++++++---
 roles/system_packages/vars/main.yml             | 14 ++++++++++++++
 scripts/assert-sorted-checksums.yml             |  7 +++++--
 tests/files/almalinux9-calico.yml               |  1 +
 tests/files/debian12-cilium.yml                 |  4 ++++
 tests/files/ubuntu24-calico-etcd-datastore.yml  |  4 ++++
 9 files changed, 53 insertions(+), 24 deletions(-)

diff --git a/roles/kubernetes/preinstall/defaults/main.yml b/roles/kubernetes/preinstall/defaults/main.yml
index 248c25f9a..c4c765444 100644
--- a/roles/kubernetes/preinstall/defaults/main.yml
+++ b/roles/kubernetes/preinstall/defaults/main.yml
@@ -55,17 +55,6 @@ minimal_node_memory_mb: 1024
 minimal_master_memory_mb: 1500
 
 ## NTP Settings
-# Start the ntpd or chrony service and enable it at system boot.
-ntp_enabled: false
-# The package to install which provides NTP functionality.
-# The default is ntp for most platforms, or chrony on RHEL/CentOS 7 and later.
-# The ntp_package can be one of ['ntp', 'ntpsec', 'chrony']
-ntp_package: >-
-      {% if ansible_os_family == "RedHat" -%}
-      chrony
-      {%- else -%}
-      ntp
-      {%- endif -%}
 
 # Manage the NTP configuration file.
 ntp_manage_config: false
diff --git a/roles/kubernetes/preinstall/tasks/0081-ntp-configurations.yml b/roles/kubernetes/preinstall/tasks/0081-ntp-configurations.yml
index 616ab26bb..7d41224a3 100644
--- a/roles/kubernetes/preinstall/tasks/0081-ntp-configurations.yml
+++ b/roles/kubernetes/preinstall/tasks/0081-ntp-configurations.yml
@@ -1,12 +1,4 @@
 ---
-- name: Ensure NTP package
-  package:
-    name:
-      - "{{ ntp_package }}"
-    state: present
-  when:
-    - not is_fedora_coreos
-    - not ansible_os_family in ["Flatcar", "Flatcar Container Linux by Kinvolk"]
 
 - name: Disable systemd-timesyncd
   service:
diff --git a/roles/kubespray_defaults/defaults/main/main.yml b/roles/kubespray_defaults/defaults/main/main.yml
index ee7a9143a..1c2039c56 100644
--- a/roles/kubespray_defaults/defaults/main/main.yml
+++ b/roles/kubespray_defaults/defaults/main/main.yml
@@ -770,3 +770,20 @@ system_upgrade_reboot: on-upgrade  # never, always
 
 # Enables or disables the scheduler plugins.
 scheduler_plugins_enabled: false
+
+## NTP Settings
+# Start the ntpd or chrony service and enable it at system boot.
+ntp_enabled: false
+
+# TODO: Refactor NTP package selection to integrate with the general package installation system
+# instead of using a separate variable approach
+
+# The package to install which provides NTP functionality.
+# The default is ntp for most platforms, or chrony on RHEL/CentOS 7 and later.
+# The ntp_package can be one of ['ntp', 'ntpsec', 'chrony']
+ntp_package: >-
+      {% if ansible_os_family == "RedHat" -%}
+      chrony
+      {%- else -%}
+      ntp
+      {%- endif -%}
diff --git a/roles/system_packages/tasks/main.yml b/roles/system_packages/tasks/main.yml
index a380fdce3..97d0cbb72 100644
--- a/roles/system_packages/tasks/main.yml
+++ b/roles/system_packages/tasks/main.yml
@@ -65,14 +65,19 @@
   tags:
     - bootstrap_os
 
-- name: Install packages requirements
+- name: Manage packages
   package:
-    name: "{{ pkgs | dict2items | selectattr('value', 'ansible.builtin.all') | map(attribute='key') }}"
-    state: present
+    name: "{{ item.packages | dict2items | selectattr('value', 'ansible.builtin.all') | map(attribute='key') }}"
+    state: "{{ item.state }}"
   register: pkgs_task_result
   until: pkgs_task_result is succeeded
   retries: "{{ pkg_install_retries }}"
   delay: "{{ retry_stagger | random + 3 }}"
   when: not (ansible_os_family in ["Flatcar", "Flatcar Container Linux by Kinvolk"] or is_fedora_coreos)
+  loop:
+    - { packages: "{{ pkgs_to_remove }}", state: "absent", action_label: "remove" }
+    - { packages: "{{ pkgs }}", state: "present", action_label: "install" }
+  loop_control:
+    label: "{{ item.action_label }}"
   tags:
     - bootstrap_os
diff --git a/roles/system_packages/vars/main.yml b/roles/system_packages/vars/main.yml
index a038fbbac..b26924047 100644
--- a/roles/system_packages/vars/main.yml
+++ b/roles/system_packages/vars/main.yml
@@ -1,4 +1,9 @@
 ---
+pkgs_to_remove:
+  systemd-timesyncd:
+    - "{{ ntp_enabled }}"
+    - "{{ ntp_package == 'ntp' }}"
+    - "{{ ansible_os_family == 'Debian' }}"
 pkgs:
   apparmor:
     - "{{ ansible_os_family == 'Debian' }}"
@@ -9,6 +14,9 @@ pkgs:
     - "{{ ansible_distribution_major_version == '10' }}"
     - "{{ 'k8s_cluster' in group_names }}"
   bash-completion: []
+  chrony:
+    - "{{ ntp_enabled }}"
+    - "{{ ntp_package == 'chrony' }}"
   conntrack:
     - "{{ ansible_os_family in ['Debian', 'RedHat'] }}"
     - "{{ ansible_distribution != 'openEuler' }}"
@@ -70,6 +78,12 @@ pkgs:
     - "{{ 'k8s_cluster' in group_names }}"
   nss:
     - "{{ ansible_os_family == 'RedHat' }}"
+  ntp:
+    - "{{ ntp_enabled }}"
+    - "{{ ntp_package == 'ntp' }}"
+  ntpsec:
+    - "{{ ntp_enabled }}"
+    - "{{ ntp_package == 'ntpsec' }}"
   openssl: []
   python-apt:
     - "{{ ansible_os_family == 'Debian' }}"
diff --git a/scripts/assert-sorted-checksums.yml b/scripts/assert-sorted-checksums.yml
index 897938bb5..d7e2e86db 100755
--- a/scripts/assert-sorted-checksums.yml
+++ b/scripts/assert-sorted-checksums.yml
@@ -40,12 +40,15 @@
     include_vars: ../roles/system_packages/vars/main.yml
 
   - name: Verify that the packages list is sorted
+    loop:
+    - pkgs_to_remove
+    - pkgs
     vars:
-      pkgs_lists: "{{ pkgs.keys() | list }}"
+      pkgs_lists: "{{ lookup('vars', item).keys() | list }}"
       ansible_distribution: irrelevant
       ansible_distribution_major_version: irrelevant
       ansible_distribution_minor_version: irrelevant
       ansible_os_family: irrelevant
     assert:
       that: "pkgs_lists | sort == pkgs_lists"
-      fail_msg: "pkgs is not sorted: {{ pkgs_lists | ansible.utils.fact_diff(pkgs_lists | sort) }}"
+      fail_msg: "{{ item }} is not sorted: {{ pkgs_lists | ansible.utils.fact_diff(pkgs_lists | sort) }}"
diff --git a/tests/files/almalinux9-calico.yml b/tests/files/almalinux9-calico.yml
index c3720b213..d3ae31619 100644
--- a/tests/files/almalinux9-calico.yml
+++ b/tests/files/almalinux9-calico.yml
@@ -14,6 +14,7 @@ kube_proxy_mode: nftables
 
 # NTP mangement
 ntp_enabled: true
+ntp_package: chrony
 ntp_timezone: Etc/UTC
 ntp_manage_config: true
 ntp_tinker_panic: true
diff --git a/tests/files/debian12-cilium.yml b/tests/files/debian12-cilium.yml
index 718f6de95..94bc3ee26 100644
--- a/tests/files/debian12-cilium.yml
+++ b/tests/files/debian12-cilium.yml
@@ -4,3 +4,7 @@ cloud_image: debian-12
 
 # Kubespray settings
 kube_network_plugin: cilium
+
+# ntp settings
+ntp_enabled: true
+ntp_package: ntp
diff --git a/tests/files/ubuntu24-calico-etcd-datastore.yml b/tests/files/ubuntu24-calico-etcd-datastore.yml
index 73bf5ef95..2a83f2c6a 100644
--- a/tests/files/ubuntu24-calico-etcd-datastore.yml
+++ b/tests/files/ubuntu24-calico-etcd-datastore.yml
@@ -44,3 +44,7 @@ kubeadm_patches:
           example.com/test: "false"
         labels:
           example.com/prod_level: "prep"
+
+# ntp settings
+ntp_enabled: true
+ntp_package: ntpsec
-- 
GitLab