migrations: Correct handling of local charm URLs #6901

Merged
merged 2 commits into from Feb 2, 2017

Conversation

Projects
None yet
5 participants
Contributor

mjs commented Feb 2, 2017

Description of change

There was a recent change to how charm URLs are tracked but migrations weren't updated to correctly support it.

Charms are now uploaded in order. This is important to ensure charm URLs are correctly assigned in the target. The charm URL used in the target is now also checked to ensure the correct value was used.

Charm revision sequences are now not directly migrated. Charm revision sequences shouldn't be explicitly set during the migration process. They get set when the charm binaries are uploaded.

QA steps

Bootstrap 2 controllers. In a model in the source controller, deploy three revisions of the same local charm. Each revision sets a slightly different status message.

$ juju deploy ~/ubuntu-debug first
# Edit status message
$ juju deploy ~/ubuntu-debug second
# Edit status message
$ juju deploy ~/ubuntu-debug second

# Observe that each charm is deployed and has reports a unique status message.

# Migrate
$ juju migrate default B

# Now check the target
$ juju switch B:default
$ juju status

# Observe that the units are still reporting the expected status messages.

# Now check that the charms are definitely assigned correctly.
$ juju add-unit first
$ juju add-unit second
$ juju add-unit third

Once the new units are up, confirmed that the status messages reported match their charm revisions.

Bug reference

https://bugs.launchpad.net/juju/+bug/1660877

@mjs mjs changed the title from 1660877 charm url migration fix to migrations: Correct handling of local charm URLs Feb 2, 2017

state/migration_import.go
- Name: key,
- Counter: value,
- })
+ // The sequences for track charm revisions aren't imported

Looks good to me!

migration/migration.go
@@ -183,6 +184,10 @@ func streamThroughTempFile(r io.Reader) (_ io.ReadSeeker, cleanup func(), err er
}
func uploadCharms(config UploadBinariesConfig) error {
+ // It is critical that charms are uploaded in ascending charm URL
+ // order due to the way local charm URLs are tracked.
+ utils.SortStringsNaturally(config.Charms)
@babbageclunk

babbageclunk Feb 2, 2017

Member

Why is this? (Reads ahead.)
I think it's so if there are multiple revisions of the same charm they're uploaded in revision order, so that the revisions are the same in the source and target? It'd be good to have that detail in the comment.

@mjs

mjs Feb 2, 2017

Contributor

Your thinking is spot on. I'll clarify the comment.

mjs added some commits Feb 2, 2017

migration: Ensure charms are uploaded in order
This is important to ensure charm URLs are correctly assigned in the
target. The charm URL used in the target is now also checked to ensure
the correct value was used.
state: Don't migrate charm revision sequences
Charm revision sequences shouldn't be explicitly set during the
migration process. They get set when the charm binaries are uploaded.

Fixes https://bugs.launchpad.net/juju/+bug/1660877
Contributor

mjs commented Feb 2, 2017

$$merge$$

Contributor

jujubot commented Feb 2, 2017

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

Contributor

jujubot commented Feb 2, 2017

Build failed: Does not match ['fixes-1660877']
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/10172

Member

axw commented Feb 2, 2017

$$fixes-1660877$$

Contributor

jujubot commented Feb 2, 2017

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot jujubot merged commit 64be732 into juju:2.1 Feb 2, 2017

@mjs mjs deleted the mjs:1660877-charm-url-migration-fix branch Feb 2, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment