-
Notifications
You must be signed in to change notification settings - Fork 2
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
set env variables to execute commands #49
Conversation
Everything seems to work fine except the remove action which does not remove the connection from the macadam-connections.json file and so we are unable to recreate it later as it still detect an existing instance |
I think i had some dirty env bc now it works perfectly |
|
||
connsFile := filepath.Join(filepath.Dir(path), connectionsFile) | ||
// set the path used for storing connection of macadam vms | ||
err = os.Setenv("PODMAN_CONNECTIONS_CONF", connsFile) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we'll be able to remove the use of connections files in macadam, I don't think we need them (just a comment for the future, nothing actionable right now)
pkg/config/config.go
Outdated
// userConfigPath returns the path to the users local config that is | ||
// not shared with other users. It uses $XDG_CONFIG_HOME/containers... | ||
// if set or $HOME/.config/containers... if not. | ||
func UserConfigPath() (string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you considered using https://github.com/containers/storage/tree/main/pkg/homedir for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated. I copy/pasted this func from the podman common lib. I removed everything in favor of homedir. I agree the connections file would not be very useful in our case so it is just a temporary way to set it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
It's missing a go module update, but this can only be done after the cfergeau/podman PR is merged.
updated with the missing go modules |
before executing any action we need to update the environment variables PODMAN_CONNECTIONS_CONF, PODMAN_DATA_DIR and PODMAN_RUNTIME_DIR to define a specific location where to store macadam data. This way we prevent to write macadam stuff within podman-related folders Signed-off-by: lstocchi <[email protected]>
@cfergeau rebased so the lint succeed |
before executing any action we need to update the environment variables PODMAN_CONNECTIONS_CONF, PODMAN_DATA_DIR and PODMAN_RUNTIME_DIR to define a specific location where to store macadam data. This way we prevent to write macadam stuff within podman-related folders
This works with cfergeau/podman#1