Update host-upgrades addon to use pharos-host-upgrades #393

SpComb · 2018-05-31T13:08:50Z

See https://github.com/kontena/pharos-host-upgrades

Currently using the quay.io/kontena/pharos-host-upgrades:edge build, pending testing and a tagged release.

The defaults run upgrades at midnight, ~~waiting up to two hours for the kube lock~~, without rebooting the host.

Replaces the kured addon, which needs to be deprecated somehow... This functionality is optional, it needs reboot: true to enable it.

TODO

Test
Don't run upgrades at exactly midnight by default
Use tagged pharos-host-upgrades release images
Deprecate kured addon #420 Deprecate the kured addon
Add arm64 support
- Missing arm64 builds pharos-host-upgrades#24

…nfiguring a schedule

SpComb · 2018-05-31T13:40:35Z

addons/host-upgrades/resources/40-configmap.yml

+    # minimal                            = yum --bugfix update-minimal
+    # minimal-security                   = yum --security update-minimal
+    # minimal-security-severity:Critical =  --sec-severity=Critical update-minimal
+    update_cmd = default


I believe this will install all available updated package from all yum repos... my understanding is that CentOS is missing the required repo metadata for things like minimal-security to work, those are only available in RHEL:

https://www.reddit.com/r/CentOS/comments/7wrjcp/how_to_handle_security_upgrades/

https://updateinfo.cefs.steve-meier.de/

https://www.caseylabs.com/centos-automatic-security-updates-do-not-work/

Yes, CentOS is missing security metadata. We lock kube/docker packages with yum-lockversion which should make this a bit more safe. For RHEL we could use security or similar?

Should probably still consider testing the combination of yum-lockversion and the yum-cron configuration to make sure it doesn't upgrade pinned packages.

Tested, yum-versionlock works with yum-cron.

SpComb · 2018-05-31T13:42:48Z

addons/host-upgrades/resources/40-configmap.yml

+
+    // Automatically upgrade packages from these (origin:archive) pairs
+    Unattended-Upgrade::Allowed-Origins {
+    // "${distro_id}:${distro_codename}";


The Ubuntu:xenial origin is whitelisted in the default configuration, AFAIK that corresponds to LTS point releases (16.04.x):

// 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"; };

jakolehm · 2018-06-18T09:49:33Z

addons/host-upgrades/addon.rb

@@ -1,20 +1,25 @@
 # frozen_string_literal: true

 Pharos.addon 'host-upgrades' do
-  version '0.1.0'
+  version '0.0.0'


Just a reminder that this need to be updated.

SpComb · 2018-06-18T11:53:00Z

addons/host-upgrades/addon.rb

  license 'Apache License 2.0'

  config {
-    attribute :interval, Pharos::Types::String
+    attribute :image_tag, Pharos::Types::String.default("edge")
+    attribute :schedule, Pharos::Types::String


This absolutely needs to have some regexp for validation, because anyone configuring this is going to be more likely to get the syntax wrong rather than right, and then be left debugging their CrashLoopBackoff pods... the feedback cycle for fixing this configuration is too long and painful otherwise.

I would prefer to merge this sooner than later.. if regexp takes time then we should open a new issue and mark it to the 1.2.0 milestone.

No need, I already have something, it just needs specs to test it.

SpComb · 2018-06-18T14:15:16Z

addons/host-upgrades/addon.rb

-    attribute :interval, Pharos::Types::String
+    attribute :image_tag, Pharos::Types::String.default("edge")
+    attribute :schedule, Pharos::Types::String
+    attribute :schedule_window, Pharos::Types::String


Note: The --schedule-window defaults to 1h within pharos-host-upgrades itself, which might be a mistake... it should still be possible to disable it by using schedule_window: 0 - or something else that makes sense given the schedule, like schedule_window: 24h.

This will now get overridden with --schedule-window=0 by default, which is IMO good behavior.

SpComb · 2018-06-18T16:12:23Z

Leaving #420 as a separate issue for the kured deprecation... having both kured and host-upgrades' running with reboot: true` may or may not behave badly.

jakolehm · 2018-06-18T16:15:25Z

addons/host-upgrades/addon.rb

+      DURATION_RE = /^(\d+(\.\d+)?(ns|us|ms|s|m|h|))+$/
+
+      def schedule?(value)
+        !!SCHEDULE_RE.match?(value)


Just wondering should we use valid cron schedule (we have fugit gem as a dep)?

The pharos-cluster itself doesn't need to actually parse the cron string, but Fugit::Cron.parse would probably be better than a regexp... unfortunately, the github.com/robfig/cron package used by pharos-host-upgrades doesn't accept the standard five-field cron syntax, it expects an extended cron syntax with a leading seconds field... which is a bad idea IMO, it would be better if this used a standard cron syntax, we don't really need per-second granularity here.

Maybe we should change pharos-host-upgrades to accept a standard five-field cron syntax before releasing this?

I meant that we could use Fugit::Cron.parse to parse standard five-field cron and then addon could convert it to something that pharos-host-upgrades understands.

I think that conversion should happen within pharos-host-upgrades.

kontena/pharos-host-upgrades#30

Looks like Fugit::Cron.parse also supports the alternative six-field cron syntax, which confuses things a bit now...

irb(main):003:0> Fugit::Cron.parse('0 0 0 * * *') => #<Fugit::Cron:0x0000000001c172c0 @original="0 0 0 * * *", @cron_s=nil, @seconds=[0], @minutes=[0], @hours=[0], @monthdays=nil, @months=nil, @weekdays=nil, @zone=nil, @timezone=nil>

jakolehm · 2018-06-18T16:17:41Z

addons/host-upgrades/resources/40-configmap.yml

+    # minimal                            = yum --bugfix update-minimal
+    # minimal-security                   = yum --security update-minimal
+    # minimal-security-severity:Critical =  --sec-severity=Critical update-minimal
+    update_cmd = default


Yes, CentOS is missing security metadata. We lock kube/docker packages with yum-lockversion which should make this a bit more safe. For RHEL we could use security or similar?

SpComb · 2018-06-19T07:13:28Z

addons/host-upgrades/addon.rb

@@ -5,16 +5,48 @@
  license 'Apache License 2.0'

  config {
-    attribute :interval, Pharos::Types::String


If we wanted to make the 1.2 upgrade smoother, we could continue to accept existing addon configurations with interval, and just translate that to schedule: "@every ...".

Not sure if it's needed if the new addon complains when schedule is missing?

Everyone using the existing host-upgrades addon will need to edit their cluster.yml when upgrading to 1.2. That could be avoided.

I think it's ok to edit cluster.yml in this case.

SpComb · 2018-06-19T14:20:59Z

spec/pharos/addons/host_upgrades_spec.rb

+      it "fails with long cron schedule" do
+        result = described_class.validate({enabled: true, schedule: '0 0 0 * * *'})
+
+        expect(result).to_not be_success


Having a hard time figuring out how to get Fugit::Cron.parse to reject or normalize the extended seconds syntax without monkey-patching it...

1) Pharos::Addons::HostUpgrades#validate schedule fails with long cron schedule Failure/Error: expect(result).to_not be_success expected `#<Dry::Validation::Result output={:enabled=>true, :schedule=>"0 0 0 * * *"} errors={}>.success?` to return false, got true # ./spec/pharos/addons/host_upgrades_spec.rb:49:in `block (4 levels) in <top (required)>' # ./spec/spec_helper.rb:121:in `block (2 levels) in <top (required)>' # ./spec/spec_helper.rb:111:in `block (2 levels) in <top (required)>'

Hacked this in with a trivial monkeypatch to add a missing attr_reader... validating/normalizing the cron schedule was more difficult than expected this way.

jakolehm · 2018-06-19T14:54:09Z

lib/pharos/addon.rb

+
+module Fugit
+  class Cron
+    attr_reader :seconds # XXX: missing from upstream


We should try to push this also to the upstream.

floraison/fugit#3

^ this got merged and released upstream as 1.1.2, so I'll still update this PR to use that instead.

jakolehm

LGTM

jakolehm · 2018-06-20T05:46:27Z

@SpComb ok to merge?

SpComb · 2018-06-20T06:31:23Z

@SpComb ok to merge?

Good to go now.

jakolehm · 2018-06-20T06:40:26Z

@SpComb docs pr is still missing...

replace host-upgrades addon with pharos-host-upgrades

4879623

SpComb added the enhancement New feature or request label May 31, 2018

remove the default schedule_window to make it less surprising when co…

0d30636

…nfiguring a schedule

jakolehm added this to the 1.2.0 milestone May 31, 2018

only drain if also reboot

e3ee1a0

SpComb commented May 31, 2018

View reviewed changes

Tero Marttila added 3 commits May 31, 2018 16:43

fix unattended-upgrades.conf to use default allowed-origins

d037665

use new per-arch images

749370a

make schedule required

eaa5946

jnummelin modified the milestones: 1.2.0, 1.1.0 Jun 5, 2018

jakolehm reviewed Jun 18, 2018

View reviewed changes

SpComb commented Jun 18, 2018

View reviewed changes

add missing config schema fields

615a291

SpComb commented Jun 18, 2018

View reviewed changes

Tero Marttila added 6 commits June 18, 2018 17:51

Merge branch 'master' into addons/pharos-host-upgrades

6924e03

typofix

4e09fbe

validate schedule

1e2bc5e

validate schedule_window duration

67596f0

rubocop fixes

bef4c6a

use tagged release versions

9d744a1

SpComb requested a review from jakolehm June 18, 2018 16:11

jakolehm reviewed Jun 18, 2018

View reviewed changes

SpComb commented Jun 19, 2018

View reviewed changes

Tero Marttila added 2 commits June 19, 2018 17:11

use 0.2.0 with standard crontab syntax

0cf34de

add spec for long cron schedule

30b6a99

normalize schedule_window duration

1d35fe2

SpComb commented Jun 19, 2018

View reviewed changes

Tero Marttila added 5 commits June 19, 2018 17:22

edit spec titles

56647c7

remove invalid duration validate specs

625b34d

normalize cron schedule

3ac166c

reject/normalize alternative seconds syntax

5170230

rubocop fixes

0999b6d

jakolehm reviewed Jun 19, 2018

View reviewed changes

jakolehm approved these changes Jun 20, 2018

View reviewed changes

update fugit for cron seconds

0486c7f

jakolehm merged commit 3db98c5 into master Jun 20, 2018

jakolehm deleted the addons/pharos-host-upgrades branch June 20, 2018 06:32

SpComb mentioned this pull request Jun 20, 2018

Document new host-upgrades addon kontena/pharos-docs#46

Merged

This was referenced Jun 26, 2018

Release v1.2.0-beta.2 #451

Merged

Release v1.2.0 #465

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update host-upgrades addon to use pharos-host-upgrades #393

Update host-upgrades addon to use pharos-host-upgrades #393

SpComb commented May 31, 2018 •

edited

Loading

SpComb May 31, 2018

jakolehm Jun 18, 2018

SpComb Jun 19, 2018

jakolehm Jun 19, 2018

SpComb May 31, 2018

jakolehm Jun 18, 2018

SpComb Jun 18, 2018

jakolehm Jun 18, 2018

SpComb Jun 18, 2018

SpComb Jun 18, 2018 •

edited

Loading

SpComb Jun 19, 2018

SpComb commented Jun 18, 2018

jakolehm Jun 18, 2018

SpComb Jun 19, 2018

jakolehm Jun 19, 2018

SpComb Jun 19, 2018

SpComb Jun 19, 2018

SpComb Jun 19, 2018

jakolehm Jun 18, 2018

SpComb Jun 19, 2018

jakolehm Jun 19, 2018

SpComb Jun 19, 2018

jakolehm Jun 19, 2018

SpComb Jun 19, 2018

SpComb Jun 19, 2018

jakolehm Jun 19, 2018

SpComb Jun 19, 2018

SpComb Jun 20, 2018

jakolehm left a comment

jakolehm commented Jun 20, 2018

SpComb commented Jun 20, 2018

jakolehm commented Jun 20, 2018

Update host-upgrades addon to use pharos-host-upgrades #393

Update host-upgrades addon to use pharos-host-upgrades #393

Conversation

SpComb commented May 31, 2018 • edited Loading

TODO

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SpComb Jun 18, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SpComb commented Jun 18, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakolehm left a comment

Choose a reason for hiding this comment

jakolehm commented Jun 20, 2018

SpComb commented Jun 20, 2018

jakolehm commented Jun 20, 2018

SpComb commented May 31, 2018 •

edited

Loading

SpComb Jun 18, 2018 •

edited

Loading