From 48796230ecefd596b4f7c01ee31911350ed0370d Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Thu, 31 May 2018 16:05:50 +0300 Subject: [PATCH 01/21] replace host-upgrades addon with pharos-host-upgrades --- addons/host-upgrades/addon.rb | 21 ++- .../resources/10-clusterrole.yml | 62 +++++++ .../resources/20-serviceaccount.yml | 5 + .../resources/30-clusterrolebinding.yml | 12 ++ .../host-upgrades/resources/40-configmap.yml | 159 ++++++++++++++++++ .../resources/50-daemonset.yml.erb | 79 +++++++++ .../host-upgrades/resources/daemonset.yml.erb | 36 ---- 7 files changed, 331 insertions(+), 43 deletions(-) create mode 100644 addons/host-upgrades/resources/10-clusterrole.yml create mode 100644 addons/host-upgrades/resources/20-serviceaccount.yml create mode 100644 addons/host-upgrades/resources/30-clusterrolebinding.yml create mode 100644 addons/host-upgrades/resources/40-configmap.yml create mode 100644 addons/host-upgrades/resources/50-daemonset.yml.erb delete mode 100644 addons/host-upgrades/resources/daemonset.yml.erb diff --git a/addons/host-upgrades/addon.rb b/addons/host-upgrades/addon.rb index c8f757751..94a3ba327 100644 --- a/addons/host-upgrades/addon.rb +++ b/addons/host-upgrades/addon.rb @@ -1,20 +1,27 @@ # frozen_string_literal: true Pharos.addon 'host-upgrades' do - version '0.1.0' + version '0.0.0' license 'Apache License 2.0' - config { - attribute :interval, Pharos::Types::String - } + IMAGE = 'quay.io/kontena/pharos-host-upgrades' - config_schema { - required(:interval).filled(:str?, :duration?) + config { + attribute :image_tag, Pharos::Types::String.default("edge") + attribute :schedule, Pharos::Types::String.default("0 0 0 * * *") + attribute :schedule_window, Pharos::Types::String.default("2h") + attribute :reboot, Pharos::Types::Bool.default(false) + attribute :drain, Pharos::Types::Bool.default(true) } install { apply_resources( - interval: duration.parse(config.interval).to_sec + image: "#{IMAGE}:#{config.image_tag}", + schedule: config.schedule, + schedule_window: config.schedule_window, + reboot: config.reboot, + drain: config.drain, + journal: false, # disabled ue to https://github.com/kontena/pharos-host-upgrades/issues/15 ) } end diff --git a/addons/host-upgrades/resources/10-clusterrole.yml b/addons/host-upgrades/resources/10-clusterrole.yml new file mode 100644 index 000000000..e9b300b47 --- /dev/null +++ b/addons/host-upgrades/resources/10-clusterrole.yml @@ -0,0 +1,62 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: host-upgrades +rules: +# locking +- apiGroups: + - apps + resources: + - daemonsets + verbs: + - get + - watch +- apiGroups: + - apps + resources: + - daemonsets + resourceNames: + - host-upgrades + verbs: + - update +- apiGroups: + - "" + resources: + - nodes + verbs: + - get +- apiGroups: + - "" + resources: + - nodes/status + verbs: + - update + +# drain +- apiGroups: + - "" + resources: + - nodes + verbs: + - update + - patch +- apiGroups: + - "" + resources: + - pods + verbs: + - list + - get + - delete +- apiGroups: + - extensions + resources: + - daemonsets + verbs: + - get +- apiGroups: + - "" + resources: + - pods/eviction + verbs: + - create diff --git a/addons/host-upgrades/resources/20-serviceaccount.yml b/addons/host-upgrades/resources/20-serviceaccount.yml new file mode 100644 index 000000000..d40fa1b22 --- /dev/null +++ b/addons/host-upgrades/resources/20-serviceaccount.yml @@ -0,0 +1,5 @@ +apiVersion: v1 +kind: ServiceAccount +metadata: + namespace: kube-system + name: host-upgrades diff --git a/addons/host-upgrades/resources/30-clusterrolebinding.yml b/addons/host-upgrades/resources/30-clusterrolebinding.yml new file mode 100644 index 000000000..29cbc2ea0 --- /dev/null +++ b/addons/host-upgrades/resources/30-clusterrolebinding.yml @@ -0,0 +1,12 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: host-upgrades +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: host-upgrades +subjects: +- kind: ServiceAccount + namespace: kube-system + name: host-upgrades diff --git a/addons/host-upgrades/resources/40-configmap.yml b/addons/host-upgrades/resources/40-configmap.yml new file mode 100644 index 000000000..9ff4f2214 --- /dev/null +++ b/addons/host-upgrades/resources/40-configmap.yml @@ -0,0 +1,159 @@ +kind: ConfigMap +apiVersion: v1 +metadata: + namespace: kube-system + name: host-upgrades + labels: + app: host-upgrades +data: + yum-cron.conf: | + [commands] + # What kind of update to use: + # default = yum upgrade + # security = yum --security upgrade + # security-severity:Critical = yum --sec-severity=Critical upgrade + # minimal = yum --bugfix update-minimal + # minimal-security = yum --security update-minimal + # minimal-security-severity:Critical = --sec-severity=Critical update-minimal + update_cmd = default + + # Whether a message should be emitted when updates are available, + # were downloaded, or applied. + update_messages = yes + + # Whether updates should be downloaded when they are available. + download_updates = yes + + # Whether updates should be applied when they are available. Note + # that download_updates must also be yes for the update to be applied. + apply_updates = yes + + # Maximum amout of time to randomly sleep, in minutes. The program + # will sleep for a random amount of time between 0 and random_sleep + # minutes before running. This is useful for e.g. staggering the + # times that multiple systems will access update servers. If + # random_sleep is 0 or negative, the program will run immediately. + # 6*60 = 360 + random_sleep = 0 + + [emitters] + # Name to use for this system in messages that are emitted. If + # system_name is None, the hostname will be used. + system_name = None + + # How to send messages. Valid options are stdio and email. If + # emit_via includes stdio, messages will be sent to stdout; this is useful + # to have cron send the messages. If emit_via includes email, this + # program will send email itself according to the configured options. + # If emit_via is None or left blank, no messages will be sent. + emit_via = stdio + + # The width, in characters, that messages that are emitted should be + # formatted to. + output_width = 80 + + + [email] + # The address to send email messages from. + # NOTE: 'localhost' will be replaced with the value of system_name. + email_from = root@localhost + + # List of addresses to send messages to. + email_to = root + + # Name of the host to connect to to send email messages. + email_host = localhost + + + [groups] + # NOTE: This only works when group_command != objects, which is now the default + # List of groups to update + group_list = None + + # The types of group packages to install + group_package_types = mandatory, default + + [base] + # This section overrides yum.conf + + # Use this to filter Yum core messages + # -4: critical + # -3: critical+errors + # -2: critical+errors+warnings (default) + debuglevel = 1 + + # skip_broken = True + mdpolicy = group:main + + # Uncomment to auto-import new gpg keys (dangerous) + # assumeyes = True + + unattended-upgrades.conf: | + // Override system /etc/apt/apt.conf.d/50unattended-upgrades + #clear "Unattended-Upgrade::Allowed-Origins"; + + // Automatically upgrade packages from these (origin:archive) pairs + Unattended-Upgrade::Allowed-Origins { + // "${distro_id}:${distro_codename}"; + "${distro_id}:${distro_codename}-security"; + // Extended Security Maintenance; doesn't necessarily exist for + // every release and this system may not have it installed, but if + // available, the policy for updates is such that unattended-upgrades + // should also install from here by default. + "${distro_id}ESM:${distro_codename}"; + // "${distro_id}:${distro_codename}-updates"; + // "${distro_id}:${distro_codename}-proposed"; + // "${distro_id}:${distro_codename}-backports"; + }; + + // List of packages to not update (regexp are supported) + Unattended-Upgrade::Package-Blacklist { + // "vim"; + // "libc6"; + // "libc6-dev"; + // "libc6-i686"; + }; + + // This option allows you to control if on a unclean dpkg exit + // unattended-upgrades will automatically run + // dpkg --force-confold --configure -a + // The default is true, to ensure updates keep getting installed + //Unattended-Upgrade::AutoFixInterruptedDpkg "false"; + + // Split the upgrade into the smallest possible chunks so that + // they can be interrupted with SIGUSR1. This makes the upgrade + // a bit slower but it has the benefit that shutdown while a upgrade + // is running is possible (with a small delay) + //Unattended-Upgrade::MinimalSteps "true"; + + // Install all unattended-upgrades when the machine is shuting down + // instead of doing it in the background while the machine is running + // This will (obviously) make shutdown slower + //Unattended-Upgrade::InstallOnShutdown "true"; + + // Send email to this address for problems or packages upgrades + // If empty or unset then no email is sent, make sure that you + // have a working mail setup on your system. A package that provides + // 'mailx' must be installed. E.g. "user@example.com" + //Unattended-Upgrade::Mail "root"; + + // Set this value to "true" to get emails only on errors. Default + // is to always send a mail if Unattended-Upgrade::Mail is set + //Unattended-Upgrade::MailOnlyOnError "true"; + + // Do automatic removal of new unused dependencies after the upgrade + // (equivalent to apt-get autoremove) + Unattended-Upgrade::Remove-Unused-Dependencies "true"; + + // Automatically reboot *WITHOUT CONFIRMATION* + // if the file /var/run/reboot-required is found after the upgrade + //Unattended-Upgrade::Automatic-Reboot "false"; + + // If automatic reboot is enabled and needed, reboot at the specific + // time instead of immediately + // Default: "now" + //Unattended-Upgrade::Automatic-Reboot-Time "02:00"; + + // Use apt bandwidth limit feature, this example limits the download + // speed to 70kb/sec + //Acquire::http::Dl-Limit "70"; diff --git a/addons/host-upgrades/resources/50-daemonset.yml.erb b/addons/host-upgrades/resources/50-daemonset.yml.erb new file mode 100644 index 000000000..a1d6b669c --- /dev/null +++ b/addons/host-upgrades/resources/50-daemonset.yml.erb @@ -0,0 +1,79 @@ +apiVersion: extensions/v1beta1 +kind: DaemonSet +metadata: + name: host-upgrades + namespace: kube-system + labels: + app: host-upgrades +spec: + updateStrategy: + type: RollingUpdate + template: + metadata: + labels: + app: host-upgrades + spec: + serviceAccountName: host-upgrades + containers: + - name: host-upgrades + image: "<%= image %>" + imagePullPolicy: IfNotPresent + command: + - pharos-host-upgrades + args: + - "--schedule=<%= schedule %>" + <% if schedule_window %> + - "--schedule-window=<%= schedule_window %>" + <% end %> + <% if reboot %> + - --reboot + <% end %> + <% if drain %> + - --drain + <% end %> + env: + - name: KUBE_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace + - name: KUBE_DAEMONSET + value: host-upgrades + - name: KUBE_NODE + valueFrom: + fieldRef: + fieldPath: spec.nodeName + securityContext: + privileged: true + volumeMounts: + - name: config + mountPath: /etc/host-upgrades + - name: host + mountPath: /run/host-upgrades + - name: dbus + mountPath: /var/run/dbus + <% if journal %> + - name: journal + mountPath: /run/log/journal + <% end %> + volumes: + - name: config + configMap: + name: host-upgrades + - name: host + hostPath: + path: /run/host-upgrades + type: DirectoryOrCreate + - name: dbus + hostPath: + path: /var/run/dbus + type: Directory + <% if journal %> + - name: journal + hostPath: + path: /var/log/journal + type: Directory + <% end %> + restartPolicy: Always + tolerations: + - effect: NoSchedule + operator: Exists diff --git a/addons/host-upgrades/resources/daemonset.yml.erb b/addons/host-upgrades/resources/daemonset.yml.erb deleted file mode 100644 index 4cb97d2e2..000000000 --- a/addons/host-upgrades/resources/daemonset.yml.erb +++ /dev/null @@ -1,36 +0,0 @@ -apiVersion: extensions/v1beta1 -kind: DaemonSet -metadata: - name: host-upgrades - namespace: kube-system -spec: - updateStrategy: - type: RollingUpdate - template: - metadata: - labels: - name: host-upgrades - spec: - hostPID: true - containers: - - name: host-upgrades - image: "docker.io/<%= arch.name == 'amd64' ? 'debian' : 'arm64v8/debian' %>:9" - imagePullPolicy: IfNotPresent - command: - - nsenter - args: - - --target=1 - - --mount - - --uts - - --net - - --pid - - -- - - sh - - -c - - "while true; sleep <%= interval %>; do /usr/bin/unattended-upgrade -d; done" - securityContext: - privileged: true - restartPolicy: Always - tolerations: - - effect: NoSchedule - operator: Exists \ No newline at end of file From 0d3063644d6f30ed1af1bd030c2981065c20ba85 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Thu, 31 May 2018 16:13:54 +0300 Subject: [PATCH 02/21] remove the default schedule_window to make it less surprising when configuring a schedule --- addons/host-upgrades/addon.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/addons/host-upgrades/addon.rb b/addons/host-upgrades/addon.rb index 94a3ba327..c3bea9309 100644 --- a/addons/host-upgrades/addon.rb +++ b/addons/host-upgrades/addon.rb @@ -9,7 +9,7 @@ config { attribute :image_tag, Pharos::Types::String.default("edge") attribute :schedule, Pharos::Types::String.default("0 0 0 * * *") - attribute :schedule_window, Pharos::Types::String.default("2h") + attribute :schedule_window, Pharos::Types::String attribute :reboot, Pharos::Types::Bool.default(false) attribute :drain, Pharos::Types::Bool.default(true) } From e3ee1a0b2ba83ea033dadc82ebb1884967b8ed52 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Thu, 31 May 2018 16:36:47 +0300 Subject: [PATCH 03/21] only drain if also reboot --- addons/host-upgrades/addon.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/addons/host-upgrades/addon.rb b/addons/host-upgrades/addon.rb index c3bea9309..1d93811dc 100644 --- a/addons/host-upgrades/addon.rb +++ b/addons/host-upgrades/addon.rb @@ -20,7 +20,7 @@ schedule: config.schedule, schedule_window: config.schedule_window, reboot: config.reboot, - drain: config.drain, + drain: config.reboot && config.drain, journal: false, # disabled ue to https://github.com/kontena/pharos-host-upgrades/issues/15 ) } From d037665b73cb2c3f2d75ad1c7f3598a149155e46 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Thu, 31 May 2018 16:43:23 +0300 Subject: [PATCH 04/21] fix unattended-upgrades.conf to use default allowed-origins --- addons/host-upgrades/resources/40-configmap.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/addons/host-upgrades/resources/40-configmap.yml b/addons/host-upgrades/resources/40-configmap.yml index 9ff4f2214..28cb2da90 100644 --- a/addons/host-upgrades/resources/40-configmap.yml +++ b/addons/host-upgrades/resources/40-configmap.yml @@ -94,7 +94,7 @@ data: // Automatically upgrade packages from these (origin:archive) pairs Unattended-Upgrade::Allowed-Origins { - // "${distro_id}:${distro_codename}"; + "${distro_id}:${distro_codename}"; "${distro_id}:${distro_codename}-security"; // Extended Security Maintenance; doesn't necessarily exist for // every release and this system may not have it installed, but if From 749370a60e71db0d7afb9ccfd11488e0bacb68d8 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Fri, 1 Jun 2018 14:09:02 +0300 Subject: [PATCH 05/21] use new per-arch images --- addons/host-upgrades/addon.rb | 4 +--- addons/host-upgrades/resources/50-daemonset.yml.erb | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/addons/host-upgrades/addon.rb b/addons/host-upgrades/addon.rb index 1d93811dc..f874ca0bf 100644 --- a/addons/host-upgrades/addon.rb +++ b/addons/host-upgrades/addon.rb @@ -4,8 +4,6 @@ version '0.0.0' license 'Apache License 2.0' - IMAGE = 'quay.io/kontena/pharos-host-upgrades' - config { attribute :image_tag, Pharos::Types::String.default("edge") attribute :schedule, Pharos::Types::String.default("0 0 0 * * *") @@ -16,7 +14,7 @@ install { apply_resources( - image: "#{IMAGE}:#{config.image_tag}", + image_tag: config.image_tag, schedule: config.schedule, schedule_window: config.schedule_window, reboot: config.reboot, diff --git a/addons/host-upgrades/resources/50-daemonset.yml.erb b/addons/host-upgrades/resources/50-daemonset.yml.erb index a1d6b669c..446565750 100644 --- a/addons/host-upgrades/resources/50-daemonset.yml.erb +++ b/addons/host-upgrades/resources/50-daemonset.yml.erb @@ -16,7 +16,7 @@ spec: serviceAccountName: host-upgrades containers: - name: host-upgrades - image: "<%= image %>" + image: "quay.io/kontena/pharos-host-upgrades-<%= arch.name %>:<%= image_tag %>" imagePullPolicy: IfNotPresent command: - pharos-host-upgrades From eaa5946b315b72d545cd4d9573419ca255014239 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Fri, 1 Jun 2018 14:25:51 +0300 Subject: [PATCH 06/21] make schedule required --- addons/host-upgrades/addon.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/addons/host-upgrades/addon.rb b/addons/host-upgrades/addon.rb index f874ca0bf..bbbf2a7d7 100644 --- a/addons/host-upgrades/addon.rb +++ b/addons/host-upgrades/addon.rb @@ -6,12 +6,16 @@ config { attribute :image_tag, Pharos::Types::String.default("edge") - attribute :schedule, Pharos::Types::String.default("0 0 0 * * *") + attribute :schedule, Pharos::Types::String attribute :schedule_window, Pharos::Types::String attribute :reboot, Pharos::Types::Bool.default(false) attribute :drain, Pharos::Types::Bool.default(true) } + config_schema { + required(:schedule).filled(:str?) + } + install { apply_resources( image_tag: config.image_tag, From 615a291c0a14cb510f5902cdedf9c00e54917dfd Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Mon, 18 Jun 2018 15:27:28 +0300 Subject: [PATCH 07/21] add missing config schema fields --- addons/host-upgrades/addon.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/addons/host-upgrades/addon.rb b/addons/host-upgrades/addon.rb index bbbf2a7d7..703810473 100644 --- a/addons/host-upgrades/addon.rb +++ b/addons/host-upgrades/addon.rb @@ -13,7 +13,11 @@ } config_schema { + optional(:image_tag).filled(:str?) required(:schedule).filled(:str?) + optional(:schedule_window).filled(:str?) + optional(:reboot).filled(:bool?) + optional(:drain).filled(:bool?) } install { From 4e09fbe1ec198faf5f0b101bba7677a0cf7f30af Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Mon, 18 Jun 2018 17:54:09 +0300 Subject: [PATCH 08/21] typofix --- addons/host-upgrades/addon.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/addons/host-upgrades/addon.rb b/addons/host-upgrades/addon.rb index 703810473..683abca6c 100644 --- a/addons/host-upgrades/addon.rb +++ b/addons/host-upgrades/addon.rb @@ -27,7 +27,7 @@ schedule_window: config.schedule_window, reboot: config.reboot, drain: config.reboot && config.drain, - journal: false, # disabled ue to https://github.com/kontena/pharos-host-upgrades/issues/15 + journal: false, # disabled due to https://github.com/kontena/pharos-host-upgrades/issues/15 ) } end From 1e2bc5ef317f2a82d7010dc2bda1f0a0015e1d73 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Mon, 18 Jun 2018 18:34:43 +0300 Subject: [PATCH 09/21] validate schedule --- addons/host-upgrades/addon.rb | 16 +++++- spec/pharos/addons/host_upgrades_spec.rb | 69 ++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 1 deletion(-) create mode 100644 spec/pharos/addons/host_upgrades_spec.rb diff --git a/addons/host-upgrades/addon.rb b/addons/host-upgrades/addon.rb index 683abca6c..fa41e032b 100644 --- a/addons/host-upgrades/addon.rb +++ b/addons/host-upgrades/addon.rb @@ -13,8 +13,22 @@ } config_schema { + configure do + SCHEDULE_RE = /^([*?]|[A-Z0-9\/,-]+)(\s+([*?]|[A-Z0-9\/,-]+)){5}|@\w+$/ + + def schedule?(value) + !!SCHEDULE_RE.match?(value) + end + + def self.messages + super.merge( + en: { errors: { schedule?: 'is not a valid cron schedule: https://github.com/kontena/pharos-host-upgrades#--schedule' } } + ) + end + end + optional(:image_tag).filled(:str?) - required(:schedule).filled(:str?) + required(:schedule).filled(:str?, :schedule?) optional(:schedule_window).filled(:str?) optional(:reboot).filled(:bool?) optional(:drain).filled(:bool?) diff --git a/spec/pharos/addons/host_upgrades_spec.rb b/spec/pharos/addons/host_upgrades_spec.rb new file mode 100644 index 000000000..0811ca238 --- /dev/null +++ b/spec/pharos/addons/host_upgrades_spec.rb @@ -0,0 +1,69 @@ +require 'pharos/addon' +require "./addons/host-upgrades/addon" + +describe Pharos::Addons::HostUpgrades do + let(:cluster_config) { Pharos::Config.load( + hosts: [ {role: 'worker'} ], + ) } + let(:config) { { } } + let(:cpu_arch) { double(:cpu_arch ) } + let(:master) { double(:host, address: '192.0.2.1') } + + subject do + described_class.new({enabled: true}.merge(config), enabled: true, master: master, cpu_arch: cpu_arch, cluster_config: cluster_config) + end + + describe "#validate" do + it "fails with missing schedule" do + result = described_class.validate({enabled: true}) + + expect(result).to_not be_success + expect(result.errors.dig(:schedule)).to eq ["is missing"] + end + + it "fails with empty schedule" do + result = described_class.validate({enabled: true, schedule: ''}) + + expect(result).to_not be_success + expect(result.errors.dig(:schedule)).to eq ["must be filled"] + end + + it "fails with invalid schedule" do + result = described_class.validate({enabled: true, schedule: '3'}) + + expect(result).to_not be_success, result.errors + #expect(result.errors).to eq {} + end + + it "fails with invalid cron schedule" do + result = described_class.validate({enabled: true, schedule: '0 0 * * *'}) + + expect(result).to_not be_success, result.errors + #expect(result.errors).to eq {} + end + + it "succeeds with valid @ schedule" do + result = described_class.validate({enabled: true, schedule: '@daily'}) + + expect(result).to be_success, result.errors.inspect + end + + it "succeeds with valid schedule" do + result = described_class.validate({enabled: true, schedule: '0 0 0 * * *'}) + + expect(result).to be_success, result.errors.inspect + end + + it "succeeds with range/interval schedule" do + result = described_class.validate({enabled: true, schedule: '0 0 3-8/2 * * *'}) + + expect(result).to be_success, result.errors.inspect + end + + it "succeeds with weekday schedule" do + result = described_class.validate({enabled: true, schedule: '0 0 3 * * MON,WED,FRI'}) + + expect(result).to be_success, result.errors.inspect + end + end +end From 67596f00d6e601ff5b53fb6acaa6c5a40f818008 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Mon, 18 Jun 2018 18:42:21 +0300 Subject: [PATCH 10/21] validate schedule_window duration --- addons/host-upgrades/addon.rb | 12 +++- spec/pharos/addons/host_upgrades_spec.rb | 87 ++++++++++++++++-------- 2 files changed, 68 insertions(+), 31 deletions(-) diff --git a/addons/host-upgrades/addon.rb b/addons/host-upgrades/addon.rb index fa41e032b..bf46c7e02 100644 --- a/addons/host-upgrades/addon.rb +++ b/addons/host-upgrades/addon.rb @@ -15,21 +15,29 @@ config_schema { configure do SCHEDULE_RE = /^([*?]|[A-Z0-9\/,-]+)(\s+([*?]|[A-Z0-9\/,-]+)){5}|@\w+$/ + DURATION_RE = /^(\d+(\.\d+)?(ns|us|ms|s|m|h|))+$/ def schedule?(value) !!SCHEDULE_RE.match?(value) end + def duration?(value) + !!DURATION_RE.match?(value) + end + def self.messages super.merge( - en: { errors: { schedule?: 'is not a valid cron schedule: https://github.com/kontena/pharos-host-upgrades#--schedule' } } + en: { errors: { + schedule?: 'is not a valid cron schedule: https://github.com/kontena/pharos-host-upgrades#--schedule', + duration?: 'is not a valid Duration: https://golang.org/pkg/time/#ParseDuration', + } } ) end end optional(:image_tag).filled(:str?) required(:schedule).filled(:str?, :schedule?) - optional(:schedule_window).filled(:str?) + optional(:schedule_window).filled(:str?, :duration?) optional(:reboot).filled(:bool?) optional(:drain).filled(:bool?) } diff --git a/spec/pharos/addons/host_upgrades_spec.rb b/spec/pharos/addons/host_upgrades_spec.rb index 0811ca238..4e8aa750d 100644 --- a/spec/pharos/addons/host_upgrades_spec.rb +++ b/spec/pharos/addons/host_upgrades_spec.rb @@ -21,49 +21,78 @@ expect(result.errors.dig(:schedule)).to eq ["is missing"] end - it "fails with empty schedule" do - result = described_class.validate({enabled: true, schedule: ''}) + describe 'schedule' do + it "fails with empty schedule" do + result = described_class.validate({enabled: true, schedule: ''}) - expect(result).to_not be_success - expect(result.errors.dig(:schedule)).to eq ["must be filled"] - end + expect(result).to_not be_success + expect(result.errors.dig(:schedule)).to eq ["must be filled"] + end - it "fails with invalid schedule" do - result = described_class.validate({enabled: true, schedule: '3'}) + it "fails with invalid schedule" do + result = described_class.validate({enabled: true, schedule: '3'}) - expect(result).to_not be_success, result.errors - #expect(result.errors).to eq {} - end + expect(result).to_not be_success + expect(result.errors.dig(:schedule)).to match [/is not a valid cron schedule/] + end - it "fails with invalid cron schedule" do - result = described_class.validate({enabled: true, schedule: '0 0 * * *'}) + it "fails with invalid cron schedule" do + result = described_class.validate({enabled: true, schedule: '0 0 * * *'}) - expect(result).to_not be_success, result.errors - #expect(result.errors).to eq {} - end + expect(result).to_not be_success + expect(result.errors.dig(:schedule)).to match [/is not a valid cron schedule/] + end - it "succeeds with valid @ schedule" do - result = described_class.validate({enabled: true, schedule: '@daily'}) + it "succeeds with valid @ schedule" do + result = described_class.validate({enabled: true, schedule: '@daily'}) - expect(result).to be_success, result.errors.inspect - end + expect(result).to be_success, result.errors.inspect + end - it "succeeds with valid schedule" do - result = described_class.validate({enabled: true, schedule: '0 0 0 * * *'}) + it "succeeds with valid schedule" do + result = described_class.validate({enabled: true, schedule: '0 0 0 * * *'}) - expect(result).to be_success, result.errors.inspect - end + expect(result).to be_success, result.errors.inspect + end + + it "succeeds with range/interval schedule" do + result = described_class.validate({enabled: true, schedule: '0 0 3-8/2 * * *'}) - it "succeeds with range/interval schedule" do - result = described_class.validate({enabled: true, schedule: '0 0 3-8/2 * * *'}) + expect(result).to be_success, result.errors.inspect + end - expect(result).to be_success, result.errors.inspect + it "succeeds with weekday schedule" do + result = described_class.validate({enabled: true, schedule: '0 0 3 * * MON,WED,FRI'}) + + expect(result).to be_success, result.errors.inspect + end end - it "succeeds with weekday schedule" do - result = described_class.validate({enabled: true, schedule: '0 0 3 * * MON,WED,FRI'}) + describe 'duration' do + it "fails with invalid duration" do + result = described_class.validate({enabled: true, schedule: '0 0 0 * * *', schedule_window: '1 hour'}) + + expect(result).to_not be_success + expect(result.errors.dig(:schedule_window)).to match [/is not a valid Duration/] + end + + it "succeeds with a zero duration" do + result = described_class.validate({enabled: true, schedule: '0 0 0 * * *', schedule_window: '0'}) + + expect(result).to be_success, result.errors.inspect + end + + it "succeeds with a simple duration" do + result = described_class.validate({enabled: true, schedule: '0 0 0 * * *', schedule_window: '1h'}) + + expect(result).to be_success, result.errors.inspect + end + + it "succeeds with a complex duration" do + result = described_class.validate({enabled: true, schedule: '0 0 0 * * *', schedule_window: '1h30m'}) - expect(result).to be_success, result.errors.inspect + expect(result).to be_success, result.errors.inspect + end end end end From bef4c6a22d7cd3902303880dd5f7fa91c3924a64 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Mon, 18 Jun 2018 18:45:05 +0300 Subject: [PATCH 11/21] rubocop fixes --- addons/host-upgrades/addon.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/addons/host-upgrades/addon.rb b/addons/host-upgrades/addon.rb index bf46c7e02..2e006976b 100644 --- a/addons/host-upgrades/addon.rb +++ b/addons/host-upgrades/addon.rb @@ -14,7 +14,7 @@ config_schema { configure do - SCHEDULE_RE = /^([*?]|[A-Z0-9\/,-]+)(\s+([*?]|[A-Z0-9\/,-]+)){5}|@\w+$/ + SCHEDULE_RE = %r(^([*?]|[A-Z0-9/,-]+)(\s+([*?]|[A-Z0-9/,-]+)){5}|@\w+$) DURATION_RE = /^(\d+(\.\d+)?(ns|us|ms|s|m|h|))+$/ def schedule?(value) @@ -29,7 +29,7 @@ def self.messages super.merge( en: { errors: { schedule?: 'is not a valid cron schedule: https://github.com/kontena/pharos-host-upgrades#--schedule', - duration?: 'is not a valid Duration: https://golang.org/pkg/time/#ParseDuration', + duration?: 'is not a valid Duration: https://golang.org/pkg/time/#ParseDuration' } } ) end From 9d744a176483b2c052cf77f53b5fd356daab655c Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Mon, 18 Jun 2018 19:09:17 +0300 Subject: [PATCH 12/21] use tagged release versions --- addons/host-upgrades/addon.rb | 5 +---- addons/host-upgrades/resources/50-daemonset.yml.erb | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/addons/host-upgrades/addon.rb b/addons/host-upgrades/addon.rb index 2e006976b..ad9f2cdbd 100644 --- a/addons/host-upgrades/addon.rb +++ b/addons/host-upgrades/addon.rb @@ -1,11 +1,10 @@ # frozen_string_literal: true Pharos.addon 'host-upgrades' do - version '0.0.0' + version '0.1.0' license 'Apache License 2.0' config { - attribute :image_tag, Pharos::Types::String.default("edge") attribute :schedule, Pharos::Types::String attribute :schedule_window, Pharos::Types::String attribute :reboot, Pharos::Types::Bool.default(false) @@ -35,7 +34,6 @@ def self.messages end end - optional(:image_tag).filled(:str?) required(:schedule).filled(:str?, :schedule?) optional(:schedule_window).filled(:str?, :duration?) optional(:reboot).filled(:bool?) @@ -44,7 +42,6 @@ def self.messages install { apply_resources( - image_tag: config.image_tag, schedule: config.schedule, schedule_window: config.schedule_window, reboot: config.reboot, diff --git a/addons/host-upgrades/resources/50-daemonset.yml.erb b/addons/host-upgrades/resources/50-daemonset.yml.erb index 446565750..f8e3ff76d 100644 --- a/addons/host-upgrades/resources/50-daemonset.yml.erb +++ b/addons/host-upgrades/resources/50-daemonset.yml.erb @@ -16,7 +16,7 @@ spec: serviceAccountName: host-upgrades containers: - name: host-upgrades - image: "quay.io/kontena/pharos-host-upgrades-<%= arch.name %>:<%= image_tag %>" + image: "quay.io/kontena/pharos-host-upgrades-<%= arch.name %>:<%= version %>" imagePullPolicy: IfNotPresent command: - pharos-host-upgrades From 0cf34de0ce2030d968451e881410a6bf3141c0c0 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Tue, 19 Jun 2018 17:11:20 +0300 Subject: [PATCH 13/21] use 0.2.0 with standard crontab syntax --- addons/host-upgrades/addon.rb | 28 +++--------------------- lib/pharos/addon.rb | 11 +++++++++- spec/pharos/addons/host_upgrades_spec.rb | 22 +++++++++---------- 3 files changed, 24 insertions(+), 37 deletions(-) diff --git a/addons/host-upgrades/addon.rb b/addons/host-upgrades/addon.rb index ad9f2cdbd..ded66ca44 100644 --- a/addons/host-upgrades/addon.rb +++ b/addons/host-upgrades/addon.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true Pharos.addon 'host-upgrades' do - version '0.1.0' + version '0.2.0' license 'Apache License 2.0' config { @@ -12,29 +12,7 @@ } config_schema { - configure do - SCHEDULE_RE = %r(^([*?]|[A-Z0-9/,-]+)(\s+([*?]|[A-Z0-9/,-]+)){5}|@\w+$) - DURATION_RE = /^(\d+(\.\d+)?(ns|us|ms|s|m|h|))+$/ - - def schedule?(value) - !!SCHEDULE_RE.match?(value) - end - - def duration?(value) - !!DURATION_RE.match?(value) - end - - def self.messages - super.merge( - en: { errors: { - schedule?: 'is not a valid cron schedule: https://github.com/kontena/pharos-host-upgrades#--schedule', - duration?: 'is not a valid Duration: https://golang.org/pkg/time/#ParseDuration' - } } - ) - end - end - - required(:schedule).filled(:str?, :schedule?) + required(:schedule).filled(:str?, :cron?) optional(:schedule_window).filled(:str?, :duration?) optional(:reboot).filled(:bool?) optional(:drain).filled(:bool?) @@ -43,7 +21,7 @@ def self.messages install { apply_resources( schedule: config.schedule, - schedule_window: config.schedule_window, + schedule_window: config.schedule_window, # only supports h, m, s reboot: config.reboot, drain: config.reboot && config.drain, journal: false, # disabled due to https://github.com/kontena/pharos-host-upgrades/issues/15 diff --git a/lib/pharos/addon.rb b/lib/pharos/addon.rb index aafb72a53..9d152074e 100644 --- a/lib/pharos/addon.rb +++ b/lib/pharos/addon.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true require 'dry-validation' +require 'fugit' + require_relative 'addons/struct' require_relative 'logging' @@ -28,9 +30,16 @@ def duration?(value) !Fugit::Duration.parse(value).nil? end + def cron?(value) + !Fugit::Cron.parse(value).nil? + end + def self.messages super.merge( - en: { errors: { duration?: 'is not valid duration' } } + en: { errors: { + duration?: 'is not valid duration', + cron?: 'is not a valid crontab' + } } ) end end diff --git a/spec/pharos/addons/host_upgrades_spec.rb b/spec/pharos/addons/host_upgrades_spec.rb index 4e8aa750d..2f902a8c1 100644 --- a/spec/pharos/addons/host_upgrades_spec.rb +++ b/spec/pharos/addons/host_upgrades_spec.rb @@ -33,14 +33,14 @@ result = described_class.validate({enabled: true, schedule: '3'}) expect(result).to_not be_success - expect(result.errors.dig(:schedule)).to match [/is not a valid cron schedule/] + expect(result.errors.dig(:schedule)).to match [/is not a valid crontab/] end it "fails with invalid cron schedule" do - result = described_class.validate({enabled: true, schedule: '0 0 * * *'}) + result = described_class.validate({enabled: true, schedule: '0 * * *'}) expect(result).to_not be_success - expect(result.errors.dig(:schedule)).to match [/is not a valid cron schedule/] + expect(result.errors.dig(:schedule)).to match [/is not a valid crontab/] end it "succeeds with valid @ schedule" do @@ -50,19 +50,19 @@ end it "succeeds with valid schedule" do - result = described_class.validate({enabled: true, schedule: '0 0 0 * * *'}) + result = described_class.validate({enabled: true, schedule: '0 0 * * *'}) expect(result).to be_success, result.errors.inspect end it "succeeds with range/interval schedule" do - result = described_class.validate({enabled: true, schedule: '0 0 3-8/2 * * *'}) + result = described_class.validate({enabled: true, schedule: '0 3-8/2 * * *'}) expect(result).to be_success, result.errors.inspect end it "succeeds with weekday schedule" do - result = described_class.validate({enabled: true, schedule: '0 0 3 * * MON,WED,FRI'}) + result = described_class.validate({enabled: true, schedule: '0 3 * * MON,WED,FRI'}) expect(result).to be_success, result.errors.inspect end @@ -70,26 +70,26 @@ describe 'duration' do it "fails with invalid duration" do - result = described_class.validate({enabled: true, schedule: '0 0 0 * * *', schedule_window: '1 hour'}) + result = described_class.validate({enabled: true, schedule: '0 0 * * *', schedule_window: '1'}) expect(result).to_not be_success - expect(result.errors.dig(:schedule_window)).to match [/is not a valid Duration/] + expect(result.errors.dig(:schedule_window)).to match [/is not a valid duration/] end it "succeeds with a zero duration" do - result = described_class.validate({enabled: true, schedule: '0 0 0 * * *', schedule_window: '0'}) + result = described_class.validate({enabled: true, schedule: '0 0 * * *', schedule_window: '0s'}) expect(result).to be_success, result.errors.inspect end it "succeeds with a simple duration" do - result = described_class.validate({enabled: true, schedule: '0 0 0 * * *', schedule_window: '1h'}) + result = described_class.validate({enabled: true, schedule: '0 0 * * *', schedule_window: '1h'}) expect(result).to be_success, result.errors.inspect end it "succeeds with a complex duration" do - result = described_class.validate({enabled: true, schedule: '0 0 0 * * *', schedule_window: '1h30m'}) + result = described_class.validate({enabled: true, schedule: '0 0 * * *', schedule_window: '1h30m'}) expect(result).to be_success, result.errors.inspect end From 30b6a99a50e979a71eca8473dcc6bd4ab5652437 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Tue, 19 Jun 2018 17:14:47 +0300 Subject: [PATCH 14/21] add spec for long cron schedule --- spec/pharos/addons/host_upgrades_spec.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/spec/pharos/addons/host_upgrades_spec.rb b/spec/pharos/addons/host_upgrades_spec.rb index 2f902a8c1..ba89bb4f2 100644 --- a/spec/pharos/addons/host_upgrades_spec.rb +++ b/spec/pharos/addons/host_upgrades_spec.rb @@ -36,13 +36,20 @@ expect(result.errors.dig(:schedule)).to match [/is not a valid crontab/] end - it "fails with invalid cron schedule" do + it "fails with short cron schedule" do result = described_class.validate({enabled: true, schedule: '0 * * *'}) expect(result).to_not be_success expect(result.errors.dig(:schedule)).to match [/is not a valid crontab/] end + it "fails with long cron schedule" do + result = described_class.validate({enabled: true, schedule: '0 0 0 * * *'}) + + expect(result).to_not be_success + expect(result.errors.dig(:schedule)).to match [/is not a valid crontab/] + end + it "succeeds with valid @ schedule" do result = described_class.validate({enabled: true, schedule: '@daily'}) From 1d35fe29eaa43d6542dd4ebf0371d445c6812450 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Tue, 19 Jun 2018 17:15:23 +0300 Subject: [PATCH 15/21] normalize schedule_window duration --- addons/host-upgrades/addon.rb | 11 +++++- spec/pharos/addons/host_upgrades_spec.rb | 49 ++++++++++++++++-------- 2 files changed, 42 insertions(+), 18 deletions(-) diff --git a/addons/host-upgrades/addon.rb b/addons/host-upgrades/addon.rb index ded66ca44..b57874e28 100644 --- a/addons/host-upgrades/addon.rb +++ b/addons/host-upgrades/addon.rb @@ -18,10 +18,19 @@ optional(:drain).filled(:bool?) } + # @return [String] + def schedule_window + return '0' if !config.schedule_window + + s = Fugit::Duration.parse(config.schedule_window).to_sec + + "#{s}s" + end + install { apply_resources( schedule: config.schedule, - schedule_window: config.schedule_window, # only supports h, m, s + schedule_window: schedule_window, # only supports h, m, s; not D, M, Y reboot: config.reboot, drain: config.reboot && config.drain, journal: false, # disabled due to https://github.com/kontena/pharos-host-upgrades/issues/15 diff --git a/spec/pharos/addons/host_upgrades_spec.rb b/spec/pharos/addons/host_upgrades_spec.rb index ba89bb4f2..761e047fc 100644 --- a/spec/pharos/addons/host_upgrades_spec.rb +++ b/spec/pharos/addons/host_upgrades_spec.rb @@ -3,7 +3,7 @@ describe Pharos::Addons::HostUpgrades do let(:cluster_config) { Pharos::Config.load( - hosts: [ {role: 'worker'} ], + hosts: [ {role: 'master', address: '192.0.2.1'} ], ) } let(:config) { { } } let(:cpu_arch) { double(:cpu_arch ) } @@ -74,32 +74,47 @@ expect(result).to be_success, result.errors.inspect end end + end - describe 'duration' do - it "fails with invalid duration" do - result = described_class.validate({enabled: true, schedule: '0 0 * * *', schedule_window: '1'}) - - expect(result).to_not be_success - expect(result.errors.dig(:schedule_window)).to match [/is not a valid duration/] + describe '#schedule_window' do + context "by default" do + it "normalizes to zero" do + expect(subject.schedule_window).to eq '0' end + end - it "succeeds with a zero duration" do - result = described_class.validate({enabled: true, schedule: '0 0 * * *', schedule_window: '0s'}) + context "with a simple duration" do + let(:config) { { schedule_window: "1 day" } } - expect(result).to be_success, result.errors.inspect + it "normalizes to seconds" do + expect(subject.schedule_window).to eq '86400s' end + end - it "succeeds with a simple duration" do - result = described_class.validate({enabled: true, schedule: '0 0 * * *', schedule_window: '1h'}) + context "with a simple duration" do + let(:config) { { schedule_window: "1h30m" } } - expect(result).to be_success, result.errors.inspect + it "normalizes to seconds" do + expect(subject.schedule_window).to eq '5400s' end + end - it "succeeds with a complex duration" do - result = described_class.validate({enabled: true, schedule: '0 0 * * *', schedule_window: '1h30m'}) + it "handles a zero duration" do + result = described_class.validate({enabled: true, schedule: '0 0 * * *', schedule_window: '0s'}) - expect(result).to be_success, result.errors.inspect - end + expect(result).to be_success, result.errors.inspect + end + + it "handles a simple duration" do + result = described_class.validate({enabled: true, schedule: '0 0 * * *', schedule_window: '1h'}) + + expect(result).to be_success, result.errors.inspect + end + + it "handles a complex duration" do + result = described_class.validate({enabled: true, schedule: '0 0 * * *', schedule_window: '1h30m'}) + + expect(result).to be_success, result.errors.inspect end end end From 56647c7527cee1a3f92e2452ef0e3aa6a56b50d4 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Tue, 19 Jun 2018 17:22:02 +0300 Subject: [PATCH 16/21] edit spec titles --- spec/pharos/addons/host_upgrades_spec.rb | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/spec/pharos/addons/host_upgrades_spec.rb b/spec/pharos/addons/host_upgrades_spec.rb index 761e047fc..27c6eadfb 100644 --- a/spec/pharos/addons/host_upgrades_spec.rb +++ b/spec/pharos/addons/host_upgrades_spec.rb @@ -22,53 +22,53 @@ end describe 'schedule' do - it "fails with empty schedule" do + it "rejects empty schedule" do result = described_class.validate({enabled: true, schedule: ''}) expect(result).to_not be_success expect(result.errors.dig(:schedule)).to eq ["must be filled"] end - it "fails with invalid schedule" do + it "rejects invalid schedule" do result = described_class.validate({enabled: true, schedule: '3'}) expect(result).to_not be_success expect(result.errors.dig(:schedule)).to match [/is not a valid crontab/] end - it "fails with short cron schedule" do + it "rejects short cron schedule" do result = described_class.validate({enabled: true, schedule: '0 * * *'}) expect(result).to_not be_success expect(result.errors.dig(:schedule)).to match [/is not a valid crontab/] end - it "fails with long cron schedule" do + it "rejects long cron schedule" do result = described_class.validate({enabled: true, schedule: '0 0 0 * * *'}) expect(result).to_not be_success expect(result.errors.dig(:schedule)).to match [/is not a valid crontab/] end - it "succeeds with valid @ schedule" do + it "accepts valid @ schedule" do result = described_class.validate({enabled: true, schedule: '@daily'}) expect(result).to be_success, result.errors.inspect end - it "succeeds with valid schedule" do + it "accepts valid schedule" do result = described_class.validate({enabled: true, schedule: '0 0 * * *'}) expect(result).to be_success, result.errors.inspect end - it "succeeds with range/interval schedule" do + it "accepts range/interval schedule" do result = described_class.validate({enabled: true, schedule: '0 3-8/2 * * *'}) expect(result).to be_success, result.errors.inspect end - it "succeeds with weekday schedule" do + it "accepts weekday schedule" do result = described_class.validate({enabled: true, schedule: '0 3 * * MON,WED,FRI'}) expect(result).to be_success, result.errors.inspect From 625b34db2975b95b0e7bee3b69f39c3991789e3e Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Tue, 19 Jun 2018 17:22:29 +0300 Subject: [PATCH 17/21] remove invalid duration validate specs --- spec/pharos/addons/host_upgrades_spec.rb | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/spec/pharos/addons/host_upgrades_spec.rb b/spec/pharos/addons/host_upgrades_spec.rb index 27c6eadfb..c8b787279 100644 --- a/spec/pharos/addons/host_upgrades_spec.rb +++ b/spec/pharos/addons/host_upgrades_spec.rb @@ -98,23 +98,5 @@ expect(subject.schedule_window).to eq '5400s' end end - - it "handles a zero duration" do - result = described_class.validate({enabled: true, schedule: '0 0 * * *', schedule_window: '0s'}) - - expect(result).to be_success, result.errors.inspect - end - - it "handles a simple duration" do - result = described_class.validate({enabled: true, schedule: '0 0 * * *', schedule_window: '1h'}) - - expect(result).to be_success, result.errors.inspect - end - - it "handles a complex duration" do - result = described_class.validate({enabled: true, schedule: '0 0 * * *', schedule_window: '1h30m'}) - - expect(result).to be_success, result.errors.inspect - end end end From 3ac166c751e612b2e9d0227ef1a25ab96b0e43f5 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Tue, 19 Jun 2018 17:41:35 +0300 Subject: [PATCH 18/21] normalize cron schedule --- addons/host-upgrades/addon.rb | 9 +++++- spec/pharos/addons/host_upgrades_spec.rb | 36 ++++++++++++++++-------- 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/addons/host-upgrades/addon.rb b/addons/host-upgrades/addon.rb index b57874e28..6ece5647b 100644 --- a/addons/host-upgrades/addon.rb +++ b/addons/host-upgrades/addon.rb @@ -18,6 +18,13 @@ optional(:drain).filled(:bool?) } + # @return [String] + def schedule + cron = Fugit::Cron.parse(config.schedule) + + cron.to_cron_s + end + # @return [String] def schedule_window return '0' if !config.schedule_window @@ -29,7 +36,7 @@ def schedule_window install { apply_resources( - schedule: config.schedule, + schedule: schedule, schedule_window: schedule_window, # only supports h, m, s; not D, M, Y reboot: config.reboot, drain: config.reboot && config.drain, diff --git a/spec/pharos/addons/host_upgrades_spec.rb b/spec/pharos/addons/host_upgrades_spec.rb index c8b787279..0108cf6d2 100644 --- a/spec/pharos/addons/host_upgrades_spec.rb +++ b/spec/pharos/addons/host_upgrades_spec.rb @@ -49,29 +49,41 @@ expect(result).to_not be_success expect(result.errors.dig(:schedule)).to match [/is not a valid crontab/] end + end + end + + describe '#schedule' do + let(:config) { { schedule: schedule } } - it "accepts valid @ schedule" do - result = described_class.validate({enabled: true, schedule: '@daily'}) + context "with a @ schedule" do + let(:schedule) { '@daily' } - expect(result).to be_success, result.errors.inspect + it "normalizes it" do + expect(subject.schedule).to eq '0 0 * * *' end + end - it "accepts valid schedule" do - result = described_class.validate({enabled: true, schedule: '0 0 * * *'}) + context "with a simple schedule" do + let(:schedule) { '0 0 * * *' } - expect(result).to be_success, result.errors.inspect + it "passes it through" do + expect(subject.schedule).to eq schedule end + end - it "accepts range/interval schedule" do - result = described_class.validate({enabled: true, schedule: '0 3-8/2 * * *'}) + context "with a range/interval schedule" do + let(:schedule) { '0 3-8/2 * * *' } - expect(result).to be_success, result.errors.inspect + it "normalizes it" do + expect(subject.schedule).to eq '0 3,5,7 * * *' end + end - it "accepts weekday schedule" do - result = described_class.validate({enabled: true, schedule: '0 3 * * MON,WED,FRI'}) + context "with a weekday schedule" do + let(:schedule) { '0 3 * * 1,3,5' } - expect(result).to be_success, result.errors.inspect + it "normalizes it" do + expect(subject.schedule).to eq schedule end end end From 5170230e079e962789d169d40a5e5d3fec9a6641 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Tue, 19 Jun 2018 17:43:07 +0300 Subject: [PATCH 19/21] reject/normalize alternative seconds syntax --- lib/pharos/addon.rb | 11 ++++++++++- spec/pharos/addons/host_upgrades_spec.rb | 10 +++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/lib/pharos/addon.rb b/lib/pharos/addon.rb index 9d152074e..33f0c91aa 100644 --- a/lib/pharos/addon.rb +++ b/lib/pharos/addon.rb @@ -3,6 +3,10 @@ require 'dry-validation' require 'fugit' +class Fugit::Cron + attr_reader :seconds # XXX: missing from upstream +end + require_relative 'addons/struct' require_relative 'logging' @@ -31,7 +35,12 @@ def duration?(value) end def cron?(value) - !Fugit::Cron.parse(value).nil? + cron = Fugit::Cron.parse(value) + + return false if !cron + return false if cron.seconds != [0] + + true end def self.messages diff --git a/spec/pharos/addons/host_upgrades_spec.rb b/spec/pharos/addons/host_upgrades_spec.rb index 0108cf6d2..7dfcad970 100644 --- a/spec/pharos/addons/host_upgrades_spec.rb +++ b/spec/pharos/addons/host_upgrades_spec.rb @@ -44,7 +44,7 @@ end it "rejects long cron schedule" do - result = described_class.validate({enabled: true, schedule: '0 0 0 * * *'}) + result = described_class.validate({enabled: true, schedule: '30 0 0 * * *'}) expect(result).to_not be_success expect(result.errors.dig(:schedule)).to match [/is not a valid crontab/] @@ -63,6 +63,14 @@ end end + context "with a seconds schedule" do + let(:schedule) { '0 30 3 * * *' } + + it "normalizes it" do + expect(subject.schedule).to eq '30 3 * * *' + end + end + context "with a simple schedule" do let(:schedule) { '0 0 * * *' } From 0999b6d160fccb03c7c8e1dd8e1c4128e53787a8 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Tue, 19 Jun 2018 17:44:35 +0300 Subject: [PATCH 20/21] rubocop fixes --- lib/pharos/addon.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/pharos/addon.rb b/lib/pharos/addon.rb index 33f0c91aa..353391340 100644 --- a/lib/pharos/addon.rb +++ b/lib/pharos/addon.rb @@ -3,8 +3,10 @@ require 'dry-validation' require 'fugit' -class Fugit::Cron - attr_reader :seconds # XXX: missing from upstream +module Fugit + class Cron + attr_reader :seconds # XXX: missing from upstream + end end require_relative 'addons/struct' From 0486c7fa96760c9c480294cc140fbe4e2241d858 Mon Sep 17 00:00:00 2001 From: Tero Marttila Date: Wed, 20 Jun 2018 09:13:42 +0300 Subject: [PATCH 21/21] update fugit for cron seconds --- lib/pharos/addon.rb | 6 ------ pharos-cluster.gemspec | 2 +- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/lib/pharos/addon.rb b/lib/pharos/addon.rb index 353391340..2536ad475 100644 --- a/lib/pharos/addon.rb +++ b/lib/pharos/addon.rb @@ -3,12 +3,6 @@ require 'dry-validation' require 'fugit' -module Fugit - class Cron - attr_reader :seconds # XXX: missing from upstream - end -end - require_relative 'addons/struct' require_relative 'logging' diff --git a/pharos-cluster.gemspec b/pharos-cluster.gemspec index 78011d0ec..5bf039cf5 100644 --- a/pharos-cluster.gemspec +++ b/pharos-cluster.gemspec @@ -31,7 +31,7 @@ Gem::Specification.new do |spec| spec.add_runtime_dependency "dry-validation", "0.11.1" spec.add_runtime_dependency "dry-struct", "0.4.0" spec.add_runtime_dependency "deep_merge" - spec.add_runtime_dependency "fugit" + spec.add_runtime_dependency "fugit", "~> 1.1.2" spec.add_runtime_dependency "rouge", "~> 3.1" spec.add_runtime_dependency "tty-prompt", "~> 0.16"