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

MEN-2145: Split mender.conf configuration file into /etc and /data. #658

Merged
merged 1 commit into from Dec 31, 2018

Conversation

Projects
None yet
3 participants
@oleorhagen
Copy link
Contributor

oleorhagen commented Nov 26, 2018

Changelog: Title

Signed-off-by: Ole Petter ole.orhagen@cfengine.com

@oleorhagen oleorhagen force-pushed the oleorhagen:MEN-2145 branch from d5a75d0 to 266f0e6 Dec 3, 2018

@oleorhagen

This comment has been minimized.

Copy link
Contributor Author

oleorhagen commented Dec 3, 2018

Slight rewrite, but the functionality is now tested, and works like expected. Have a look at the persistent_configs variable to see what you would like in your persistent config.

@@ -122,9 +122,28 @@ python do_prepare_mender_conf() {
# Tenant-token is optional, but falls back to a default-value set in config.go
conf_maybe_add("TenantToken", "MENDER_TENANT_TOKEN", getvar=True, integer=False)
dst_conf = os.path.join(d.getVar("B"), "mender.conf")
# Split the mender.conf file into a transient and a persistent configuration file.
persistent_configs = ["RootfsPartA", "RootfsPartB"]

This comment has been minimized.

@oleorhagen

oleorhagen Dec 3, 2018

Author Contributor

Here!

# Extract the variables that are destined for the persistent mender-configuration.
for config_var in transient_conf:
if config_var in persistent_configs:
persistent_conf[config_var] = transient_conf[config_var]

This comment has been minimized.

@kacf

kacf Dec 6, 2018

Member

It just occurred to me that this is not upgrade-safe. Imagine if you have an existing device that has everything in /etc/mender/mender.conf. Its new update will have stuff split into /etc and /data, but /data is not distributed in updates, only in initial deployments. Hence the device will be missing this information once it reboots into the new update. And there is no simple workaround with the current approach.

I think we can solve this by introducing a PACKAGECONFIG option, say split-mender-conf, which defaults to on, but which can be turned off for those who need it. Essentially it would be along these lines:

# Just needs to defined. It normally deals with dependencies,
# but we don't need any here.
PACKAGECONFIG[split-mender-conf] = ",,,"

PACKAGECONFIG_append = " split-mender-conf"

# And then in every place where you need to make a decision based on it.
# Python:
bb.utils.contains('PACKAGECONFIG', 'split-mender-conf', True, False, d)
# Shell
${bb.utils.contains('PACKAGECONFIG', 'split-mender-conf', 'true', 'false', d)}

So then you simply don't do any splitting if it's not set. What do you think?

This comment has been minimized.

@kacf

kacf Dec 6, 2018

Member

This should also be mentioned in the changelog, what has happened, and how to turn it off for those who need it. And maybe even an entry in our troubleshooting guide.

This comment has been minimized.

@kacf

kacf Dec 6, 2018

Member

Perhaps @mirzak or @drewmoseley have some input also. Basically, how do we ensure that upgrading works if we start splitting mender.conf across /etc and /data.

This comment has been minimized.

@drewmoseley

drewmoseley Dec 6, 2018

Contributor

Is it possible to create the /data/ contents with a state script of some kind?

This comment has been minimized.

@kacf

kacf Dec 7, 2018

Member

That's also a possibility. One advantage of doing it that way is that we would properly migrate everyone, instead of keeping support for the old way indefinitely. We could make this as an optional script, and document the upgrade procedure.

This comment has been minimized.

@oleorhagen

oleorhagen Dec 11, 2018

Author Contributor

I feel like it should be the other way around. You have to enable this functionality, rather than disable it. Thus migrating will be an option, but strongly advised. However, I do not have any strong feelings regarding this. First taker gets the decision.

This comment has been minimized.

@kacf

kacf Dec 11, 2018

Member

From a compatibility perspective, I see your point, but I think this is too detailed that most people will care enough to enable it, so I still think that our recommendation (which is to split) should be the default.

It will of course mean that we cannot backport this, but this is rather expected I would say.

@oleorhagen oleorhagen force-pushed the oleorhagen:MEN-2145 branch 7 times, most recently from 225a164 to 3e9afec Dec 11, 2018

@oleorhagen

This comment has been minimized.

Copy link
Contributor Author

oleorhagen commented Dec 18, 2018

@drewmoseley I have added a state-script for migrating the configuration as well, for those that want to move to the new configuration management (it's [also] what we do). It has an explicit dependency upon jq. Don't know if this is acceptable or not?

@drewmoseley

This comment has been minimized.

Copy link
Contributor

drewmoseley commented Dec 18, 2018

I see no issue with requiring jq.

@kacf
Copy link
Member

kacf left a comment

Overall really nice, I think the state script approach is great!

Also a very good commit/changelog message, just the way I like it! I would just add two small details: You mention both that the feature can be disabled, and that you can do a migration, but not how, so I would just mention the PACKAGECONFIG_remove = "split-mender-config" and the mender-migrate-configuration recipe by name.

In addition to this PR I would also add an entry under our Troubleshooting section, describing the symptom that would happen if you tried to upgrade without doing either the workarounds. I suppose that would be missing RootfsPartA and RootfsPartB entries.

@oleorhagen oleorhagen force-pushed the oleorhagen:MEN-2145 branch from 3e9afec to 4394e07 Dec 20, 2018

@oleorhagen

This comment has been minimized.

Copy link
Contributor Author

oleorhagen commented Dec 20, 2018

Good eye as usual! @kacf. All your comments have now been implemented, and this should be ready to merge.

MEN-2145: Split mender.conf configuration file into /etc and /data.
Changelog: Splits the mender.conf configuration file into a transient
configuration /etc/mender/mender.conf, which holds all the transient
configuration options. Whereas in /data/mender/mender.conf, the persistent
mender configs will now reside. Note that this is now enabled by default, and
thus needs to be explicitly disabled if you do not want your configuration split
in this way. This can be disabled by adding `PACKAGECONFIG_remove =
"split-mender-configuration` to your local.conf file. There is also a recipe
added for adding a state script that will help migrate the data from the old
configuration to the new, for the customers stuck on an old setup. This is
enabled by adding `IMAGE_INSTALL_append = " mender-migrate-configuration"`.

Signed-off-by: Ole Petter <ole.orhagen@cfengine.com>

@oleorhagen oleorhagen force-pushed the oleorhagen:MEN-2145 branch from 4394e07 to 5acbed2 Dec 20, 2018

@kacf

kacf approved these changes Dec 28, 2018

@kacf

This comment has been minimized.

Copy link
Member

kacf commented Dec 28, 2018

I added a test for it as well, since I need to test the cooperation between meta-mender and mender-image-tests a bit anyway.

Tested here with the latest checksum changes as well.

@oleorhagen oleorhagen merged commit 5acbed2 into mendersoftware:master Dec 31, 2018

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