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

storage: allow to use a different configuration file #199

Merged
merged 3 commits into from
Jul 16, 2018

Conversation

giuseppe
Copy link
Member

Move the configuration file parsing to a new function ReloadConfigurationFile so that it is possible to load a differentconfiguration file and override the current settings. This is useful with rootless containers so a configuration file specific to the user can be loaded.

store.go Outdated
DefaultStoreOptions.GraphRoot = "/var/lib/containers/storage"
DefaultStoreOptions.GraphDriverName = ""

func ReloadConfigurationFile(configFile string, storeOptions *StoreOptions) {
Copy link
Member

Choose a reason for hiding this comment

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

Add a comment defining this function.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

storage.conf Outdated
@@ -25,6 +25,11 @@ additionalimagestores = [
# certain container storage drivers.
size = ""

# Path to a FUSE helper to use for mounting the file system instead of mounting it
# directly.
#fuse_program = "/usr/bin/fuse-overlayfs"
Copy link
Member

Choose a reason for hiding this comment

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

Should we call this mount_program rather then fuse. I see no reason to only do this for fuse executables.

#mount_program = "/usr/bin/fuse-overlayfs"

Copy link
Member Author

Choose a reason for hiding this comment

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

this was already present in containers/storage. So I've added a new commit to rename the option name everywhere

store.go Outdated
@@ -2992,6 +2992,9 @@ type OptionsConfig struct {

// Do not create a bind mount on the storage home
SkipMountHome string `toml:"skip_mount_home"`

// Alternative program to use for the mount of the file system
FuseProgram string `toml:"fuse_program"`
Copy link
Member

Choose a reason for hiding this comment

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

MountProgram ...

Copy link
Member Author

Choose a reason for hiding this comment

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

done, I've renamed Fuse -> Mount everywhere

Move the configuration file parsing to a new function
ReloadConfigurationFile so that it is possible to load a different
configuration file and override the current settings.  This is useful
with rootless containers so a configuration file specific to the user
can be loaded.

Signed-off-by: Giuseppe Scrivano <[email protected]>
@giuseppe giuseppe force-pushed the allow-to-override-conf-file branch from b598b50 to 4ebf4f7 Compare July 16, 2018 14:03
@rhatdan
Copy link
Member

rhatdan commented Jul 16, 2018

LGTM
@nalind PTAL


Specifies the path to a custom FUSE program to use instead for mounting the file system.
Specifies the path to a custom program to use instead for mounting the file system.
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't be in the storage.options.thinpool section.

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks, I've moved it

store.go Outdated
storeOptions.GraphDriverOptions = append(storeOptions.GraphDriverOptions, fmt.Sprintf("%s.mount_program=%s", config.Storage.Driver, config.Storage.Options.MountProgram))
}
if config.Storage.Options.OverrideKernelCheck != "" {
storeOptions.GraphDriverOptions = append(storeOptions.GraphDriverOptions, fmt.Sprintf("%s.override_kernel_check=%s", config.Storage.Driver, config.Storage.Options.OverrideKernelCheck))
}
Copy link
Member

Choose a reason for hiding this comment

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

This block duplicates the block that follows it.

Copy link
Member Author

Choose a reason for hiding this comment

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

dropped

@nalind
Copy link
Member

nalind commented Jul 16, 2018

A couple of comments, but mostly fine.

@giuseppe giuseppe force-pushed the allow-to-override-conf-file branch from 4ebf4f7 to e933db5 Compare July 16, 2018 15:22
@rhatdan rhatdan merged commit 124de68 into containers:master Jul 16, 2018
@runcom runcom mentioned this pull request Nov 9, 2018
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.

3 participants