Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Load OS version from boot partition for greater compatibility #37

Merged
merged 1 commit into from
Nov 14, 2018

Conversation

dfunckt
Copy link
Contributor

@dfunckt dfunckt commented Nov 5, 2018

This also removes the runtime dependency to balena-sdk and changes the public API of configure and initialize to require a device type manifest as an argument.

Change-type: major

@dfunckt
Copy link
Contributor Author

dfunckt commented Nov 5, 2018

Hmm, making the tests pass is going to be tricky. The images shipped for the test suite are really old and don't have os-release on the boot partition. Should I fetch more recent ones? Or maybe add the file manually and commit (though what should its contents be)? Any ideas?

@pimterry
Copy link
Contributor

pimterry commented Nov 5, 2018

I would manually edit them and try to make them representative. They're not actually real images anyway, they're manually edited ones, because otherwise the repo would be massive.

The other option ofc is that you could look at #26, but I suspect that's a fair bit more work.

@dfunckt
Copy link
Contributor Author

dfunckt commented Nov 5, 2018

@pimterry, I've updated the images by mounting their partitions and copying the file from /etc/os-release, so it's as accurate as it can get. Tests pass now, thanks :)

Copy link
Contributor

@pimterry pimterry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some small comments, but otherwise this looks like a nice simple fix.

Marking it as a major version bump so it's not picked up by really old CLIs which are probably used to provision old images which don't have os-release on the boot partition. (Please let me know if you think otherwise)

I think we can actually do this as a non-major bump, and it will improve things overall. The previous version was hacky and doesn't work in lots of cases, whilst this new version will work in a lot more cases. Also, if it doesn't work anywhere it's fine - we just configure for both, which is an acceptable result for old images, and shouldn't break anything anywhere. Happy to discuss, but personally I'd lean towards making this minor.

@dfunckt dfunckt force-pushed the version-from-img branch 2 times, most recently from 5234e71 to 2ca1238 Compare November 7, 2018 18:13
This also removes the runtime dependency to balena-sdk and changes the public API of `configure` and `initialize` to require a device type manifest as an argument.

Change-type: major
@dfunckt dfunckt requested a review from pimterry November 8, 2018 17:50
@dfunckt dfunckt merged commit 913f851 into master Nov 14, 2018
@dfunckt dfunckt deleted the version-from-img branch November 14, 2018 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants