Merge lp:~jameinel/juju-core/1.18-refuse-downgrade-1299802 into lp:juju-core/1.18

Proposed by John A Meinel
Status: Merged
Approved by: John A Meinel
Approved revision: no longer in the source branch.
Merged at revision: 2264
Proposed branch: lp:~jameinel/juju-core/1.18-refuse-downgrade-1299802
Merge into: lp:juju-core/1.18
Diff against target: 67 lines (+34/-1)
3 files modified
state/testing/agent.go (+2/-0)
worker/upgrader/upgrader.go (+10/-0)
worker/upgrader/upgrader_test.go (+22/-1)
To merge this branch: bzr merge lp:~jameinel/juju-core/1.18-refuse-downgrade-1299802
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+214878@code.launchpad.net

Commit message

worker/upgrader: Refuse to downgrade

We've always considered the possibility of rolling back an upgrade with
"juju upgrade-juju --version=$OLD", however we never actually
implemented downgrade logic. And the new upgrade steps aren't actually
reversible. Bug #1299802 happened because 1.18 changes some state on
disk vs 1.16. It also has a race condition during upgrade where a 1.16
Unit agent upgrades faster than its Machine agent. When the API server
is 1.18, it will tell the Unit agent that it should actually match its
Machine agent version, thereby causing it to try to downgrade back to
1.16. What that actually does is just roll back to the 1.16 tools which
are now incompatible with the 1.18 agent.conf file.

Since we don't actually support rollback of upgrades, this just changes
the upgrader to refuse to change version to something older than
version.Current.

https://codereview.appspot.com/85710044/

Description of the change

worker/upgrader: Refuse to downgrade

We've always considered the possibility of rolling back an upgrade with
"juju upgrade-juju --version=$OLD", however we never actually
implemented downgrade logic. And the new upgrade steps aren't actually
reversible. Bug #1299802 happened because 1.18 changes some state on
disk vs 1.16. It also has a race condition during upgrade where a 1.16
Unit agent upgrades faster than its Machine agent. When the API server
is 1.18, it will tell the Unit agent that it should actually match its
Machine agent version, thereby causing it to try to downgrade back to
1.16. What that actually does is just roll back to the 1.16 tools which
are now incompatible with the 1.18 agent.conf file.

Since we don't actually support rollback of upgrades, this just changes
the upgrader to refuse to change version to something older than
version.Current.

https://codereview.appspot.com/85710044/

To post a comment you must log in.
Revision history for this message
John A Meinel (jameinel) wrote :
Download full text (4.7 KiB)

Reviewers: mp+214878_code.launchpad.net,

Message:
Please take a look.

Description:
worker/upgrader: Refuse to downgrade

We've always considered the possibility of rolling back an upgrade with
"juju upgrade-juju --version=$OLD", however we never actually
implemented downgrade logic. And the new upgrade steps aren't actually
reversible. Bug #1299802 happened because 1.18 changes some state on
disk vs 1.16. It also has a race condition during upgrade where a 1.16
Unit agent upgrades faster than its Machine agent. When the API server
is 1.18, it will tell the Unit agent that it should actually match its
Machine agent version, thereby causing it to try to downgrade back to
1.16. What that actually does is just roll back to the 1.16 tools which
are now incompatible with the 1.18 agent.conf file.

Since we don't actually support rollback of upgrades, this just changes
the upgrader to refuse to change version to something older than
version.Current.

https://code.launchpad.net/~jameinel/juju-core/1.18-refuse-downgrade-1299802/+merge/214878

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/85710044/

Affected files (+36, -1 lines):
   A [revision details]
   M state/testing/agent.go
   M worker/upgrader/upgrader.go
   M worker/upgrader/upgrader_test.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: tarmac-20140402091849-omo4q058vgwqp3oo
+New revision: <email address hidden>

Index: state/testing/agent.go
=== modified file 'state/testing/agent.go'
--- state/testing/agent.go 2014-03-12 22:33:09 +0000
+++ state/testing/agent.go 2014-04-09 04:50:46 +0000
@@ -10,6 +10,8 @@

  // SetAgentVersion sets the current agent version in the state's
  // environment configuration.
+// This is similar to state.SetEnvironAgentVersion but it doesn't require
that
+// the environment have all agents at the same version already.
  func SetAgentVersion(st *state.State, vers version.Number) error {
   return st.UpdateEnvironConfig(map[string]interface{}{"agent-version":
vers.String()}, nil, nil)
  }

Index: worker/upgrader/upgrader.go
=== modified file 'worker/upgrader/upgrader.go'
--- worker/upgrader/upgrader.go 2014-03-21 03:27:16 +0000
+++ worker/upgrader/upgrader.go 2014-04-09 04:50:46 +0000
@@ -112,6 +112,16 @@
    case <-dying:
     return nil
    }
+ if wantVersion.Compare(version.Current.Number) < 0 {
+ // See also bug #1299802 where when upgrading from
+ // 1.16 to 1.18 there is a race condition that can
+ // cause the unit agent to upgrade, and then want to
+ // downgrade when its associate machine agent has not
+ // finished upgrading.
+ logger.Infof("desired tool version: %s is older than current %s,
refusing to downgrade",
+ wantVersion, version.Current)
+ continue
+ }
    if wantVersion != currentTools.Version.Number {
     logger.Infof("upgrade requested from %v to %v", currentTools.Version,
wantVersion)
     // TODO(dimitern) 2013-10-03 bug #1234715

Index: worker/upgrader/upgrader_test.go
=== modif...

Read more...

Revision history for this message
Dimiter Naydenov (dimitern) wrote :
Revision history for this message
Go Bot (go-bot) wrote :
Download full text (10.4 KiB)

The attempt to merge lp:~jameinel/juju-core/1.18-refuse-downgrade-1299802 into lp:juju-core/1.18 failed. Below is the output from the failed tests.

ok launchpad.net/juju-core 0.014s
ok launchpad.net/juju-core/agent 1.145s
ok launchpad.net/juju-core/agent/mongo 0.531s
ok launchpad.net/juju-core/agent/tools 0.186s
ok launchpad.net/juju-core/bzr 5.062s
ok launchpad.net/juju-core/cert 2.497s
ok launchpad.net/juju-core/charm 0.400s
? launchpad.net/juju-core/charm/hooks [no test files]
? launchpad.net/juju-core/charm/testing [no test files]
ok launchpad.net/juju-core/cloudinit 0.030s
ok launchpad.net/juju-core/cloudinit/sshinit 0.934s
ok launchpad.net/juju-core/cmd 0.188s
ok launchpad.net/juju-core/cmd/charm-admin 0.761s
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]
ok launchpad.net/juju-core/cmd/juju 200.026s
ok launchpad.net/juju-core/cmd/jujud 64.876s
ok launchpad.net/juju-core/cmd/plugins/juju-metadata 8.678s
? launchpad.net/juju-core/cmd/plugins/juju-restore [no test files]
ok launchpad.net/juju-core/cmd/plugins/local 0.196s
? launchpad.net/juju-core/cmd/plugins/local/juju-local [no test files]
ok launchpad.net/juju-core/constraints 0.024s
ok launchpad.net/juju-core/container 0.037s
ok launchpad.net/juju-core/container/factory 0.044s
ok launchpad.net/juju-core/container/kvm 0.218s
ok launchpad.net/juju-core/container/kvm/mock 0.046s
? launchpad.net/juju-core/container/kvm/testing [no test files]
ok launchpad.net/juju-core/container/lxc 4.351s
? launchpad.net/juju-core/container/lxc/mock [no test files]
? launchpad.net/juju-core/container/lxc/testing [no test files]
? launchpad.net/juju-core/container/testing [no test files]
ok launchpad.net/juju-core/downloader 5.255s
ok launchpad.net/juju-core/environs 2.373s
ok launchpad.net/juju-core/environs/bootstrap 10.138s
ok launchpad.net/juju-core/environs/cloudinit 0.462s
ok launchpad.net/juju-core/environs/config 2.351s
ok launchpad.net/juju-core/environs/configstore 0.029s
ok launchpad.net/juju-core/environs/filestorage 0.028s
ok launchpad.net/juju-core/environs/httpstorage 0.727s
ok launchpad.net/juju-core/environs/imagemetadata 0.441s
? launchpad.net/juju-core/environs/imagemetadata/testing [no test files]
ok launchpad.net/juju-core/environs/instances 0.045s
ok launchpad.net/juju-core/environs/jujutest 0.201s
ok launchpad.net/juju-core/environs/manual 13.667s
ok launchpad.net/juju-core/environs/simplestreams 0.253s
? launchpad.net/juju-core/environs/simplestreams/testing [no test files]
ok launchpad.net/juju-core/environs/sshstorage 0.910s
ok launchpad.net/juju-core/environs/storage 0.825s
ok launchpad.net/juju-core/environs/sync 44.210s
ok launchpad.net/juju-core/environs/testing 0.133s
ok launchpad.net/juju-core/environs/tools 4.771s
? launchpad.net/juju-core/environs/tools/testing [no test files]
ok launchpad.net/juju-core/errors 0.011s
ok launchpad.net/juju-core/instance 0.021s
? launchpad.net/juju-core/instance/testing [no test files]
ok launchpad.net/juju-core/juju 19.396s
ok launchpad.net/juju-core/ju...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'state/testing/agent.go'
2--- state/testing/agent.go 2014-03-12 22:33:09 +0000
3+++ state/testing/agent.go 2014-04-09 06:07:42 +0000
4@@ -10,6 +10,8 @@
5
6 // SetAgentVersion sets the current agent version in the state's
7 // environment configuration.
8+// This is similar to state.SetEnvironAgentVersion but it doesn't require that
9+// the environment have all agents at the same version already.
10 func SetAgentVersion(st *state.State, vers version.Number) error {
11 return st.UpdateEnvironConfig(map[string]interface{}{"agent-version": vers.String()}, nil, nil)
12 }
13
14=== modified file 'worker/upgrader/upgrader.go'
15--- worker/upgrader/upgrader.go 2014-03-21 03:27:16 +0000
16+++ worker/upgrader/upgrader.go 2014-04-09 06:07:42 +0000
17@@ -112,6 +112,16 @@
18 case <-dying:
19 return nil
20 }
21+ if wantVersion.Compare(version.Current.Number) < 0 {
22+ // See also bug #1299802 where when upgrading from
23+ // 1.16 to 1.18 there is a race condition that can
24+ // cause the unit agent to upgrade, and then want to
25+ // downgrade when its associate machine agent has not
26+ // finished upgrading.
27+ logger.Infof("desired tool version: %s is older than current %s, refusing to downgrade",
28+ wantVersion, version.Current)
29+ continue
30+ }
31 if wantVersion != currentTools.Version.Number {
32 logger.Infof("upgrade requested from %v to %v", currentTools.Version, wantVersion)
33 // TODO(dimitern) 2013-10-03 bug #1234715
34
35=== modified file 'worker/upgrader/upgrader_test.go'
36--- worker/upgrader/upgrader_test.go 2014-03-21 03:27:16 +0000
37+++ worker/upgrader/upgrader_test.go 2014-04-09 06:07:42 +0000
38@@ -227,7 +227,28 @@
39 // something invalid and ensure we don't actually get an error, because
40 // it doesn't actually do an HTTP request
41 u := s.makeUpgrader()
42- newTools.URL = "http://localhost:999999/invalid/path/tools.tgz"
43+ newTools.URL = "http://0.1.2.3/invalid/path/tools.tgz"
44 err := upgrader.EnsureTools(u, newTools, utils.VerifySSLHostnames)
45 c.Assert(err, gc.IsNil)
46 }
47+
48+func (s *UpgraderSuite) TestUpgraderRefusesToDowngrade(c *gc.C) {
49+ stor := s.Conn.Environ.Storage()
50+ origTools := envtesting.PrimeTools(c, stor, s.DataDir(), version.MustParseBinary("5.4.3-precise-amd64"))
51+ s.PatchValue(&version.Current, origTools.Version)
52+ downgradeTools := envtesting.AssertUploadFakeToolsVersions(
53+ c, stor, version.MustParseBinary("5.3.3-precise-amd64"))[0]
54+ err := statetesting.SetAgentVersion(s.State, downgradeTools.Version.Number)
55+ c.Assert(err, gc.IsNil)
56+
57+ u := s.makeUpgrader()
58+ err = u.Stop()
59+ // If the upgrade would have triggered, we would have gotten an
60+ // UpgradeReadyError, since it was skipped, we get no error
61+ c.Check(err, gc.IsNil)
62+ _, err = agenttools.ReadTools(s.DataDir(), downgradeTools.Version)
63+ // TODO: ReadTools *should* be returning some form of NotFoundError,
64+ // however, it just passes back a fmt.Errorf so we live with it
65+ // c.Assert(err, jc.Satisfies, errors.IsNotFoundError)
66+ c.Check(err, gc.ErrorMatches, "cannot read tools metadata in tools directory.*no such file or directory")
67+}

Subscribers

People subscribed via source and target branches

to all changes: