Skip to content
This repository has been archived by the owner on May 21, 2024. It is now read-only.

dockerapp: avoid multiple current ostree installs #1665

Merged
merged 1 commit into from
May 6, 2020

Conversation

doanac
Copy link
Collaborator

@doanac doanac commented May 4, 2020

Compare current against target OSTree hash and skip the install call if
the same.

Signed-off-by: Ricardo Salveti ricardo@foundries.io
Signed-off-by: Andy Doan andy@foundries.io

Compare current against target OSTree hash and skip the install call if
the same.

Signed-off-by: Ricardo Salveti <ricardo@foundries.io>
Signed-off-by: Andy Doan <andy@foundries.io>
@pattivacek
Copy link
Collaborator

This looks fine, but I'm a bit confused why this even happens. Shouldn't we catch this at an earlier stage during the check for updates? If a target is already installed, we shouldn't consider it a new update, and we should toss it out.

By coincidence, I was just looking at that code yesterday and will probably make a small PR to improve some things, but it won't change the basic idea here.

@mike-sul
Copy link
Collaborator

mike-sul commented May 5, 2020

Shouldn't we catch this at an earlier stage during the check for updates?

I don't think that this stage present in aktualizr-lite if you mean SotaUptaneClient::checkUpdates() what eventually calls this line

} else if (current_version->filename() != target.filename()) {
, the lite doesn't use the uptane cycle.

@doanac
Copy link
Collaborator Author

doanac commented May 5, 2020

I didn't realize this was specific to aklite. I don't mind keeping this out of tree if you'd rather do that

@pattivacek
Copy link
Collaborator

I didn't realize this was specific to aklite. I don't mind keeping this out of tree if you'd rather do that

Nah, sorry, didn't mean to imply that. I was just trying to understand why this was necessary, since we already do a similar check. I think Mike is right that this is because of the unusual nature of this package manager. It's fine for now, I think.

@doanac
Copy link
Collaborator Author

doanac commented May 5, 2020

Nah, sorry, didn't mean to imply that.

You didn't imply anything. There's a cost to ever "if" in a code base, so its always worth thinking about.

@pattivacek pattivacek merged commit 4225611 into advancedtelematic:master May 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants