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.

Sign up for GitHub

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

Jump to bottom

feat(config): make it so you can revert override of apps #440

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(config): make it so you can revert override of apps #440

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions subcommands/common_config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,14 @@ func SetUpdatesConfig(opts *SetUpdatesConfigOptions, reportedTag string, reporte
fmt.Printf("Setting apps to [%s]\n", opts.UpdateApps)
sota.Set("pacman.docker_apps", opts.UpdateApps)
sota.Set("pacman.compose_apps", opts.UpdateApps)
if strings.TrimSpace(opts.UpdateApps) == "-" {
if sota.Has("pacman.docker_apps") {
DieNotNil(sota.Delete("pacman.docker_apps"))
}
if sota.Has("pacman.compose_apps") {
DieNotNil(sota.Delete("pacman.compose_apps"))
}
}
changed = true
}
if opts.UpdateTag != "" && configuredTag != opts.UpdateTag {
Expand Down
10 changes: 9 additions & 1 deletion subcommands/config/updates.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,15 @@ in a device group. When run without options, prints out the current configuratio
fioctl config updates --group beta --apps shellhttpd

# Set the docker apps and the tag for devices:
fioctl config updates --group beta --apps shellhttpd --tag master`,
fioctl config updates --group beta --apps shellhttpd --tag master

# Set the docker apps to none for devices, this will make the device run no apps:
fioctl config updates --apps ,

# Set the docker apps to inherit from their parent, meaning:
# If for a device apps are not set - all apps are installed
  # If for a device apps are set - those apps are installed
Copy link
Member

Choose a reason for hiding this comment

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

We still aren't explain what the "parent" will be. I'm not sure parent is a good word. What we are trying to describe is how are config files are processed. We use the same logic as systemd where we walk through:

  • /usr/lib/sota/conf.d/
  • /var/sota/sota.toml
  • /etc/sota/conf.d/

We need a concise way to explain this.

And that , and - are so subtle here that we should probably call out that they are special characters.

Copy link
Member

Choose a reason for hiding this comment

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

IIUC the group level is the top one here (as we allow no factory-wide updates config).
So, probably, we may put here a sentence like this:

Suggested change
  # If for a device apps are set - those apps are installed
# Set the docker apps to a system default (all apps on most devices):

A system default seems to be rather specific to me, without diving into internal details (which can differ from device to device).

fioctl config updates --apps -`,
}
cmd.AddCommand(configUpdatesCmd)
configUpdatesCmd.Flags().StringP("group", "g", "", "Device group to use")
Expand Down
11 changes: 10 additions & 1 deletion subcommands/devices/config_updates.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,16 @@ currently configured and reporting.`,

# Set the docker apps and the tag:
fioctl devices config updates <device> --apps shellhttpd --tag master
`,

# Set the docker apps to none, meaning it will run no apps:
fioctl devices config updates <device> --apps ,

# Set the docker apps to inherit from their parent, meaning:
# If for a device apps are not set - all apps are installed
#  If for a device apps are set* - only those apps are installed
#
# * It can be set at the individual device level, or at a device group level
fioctl devices config updates <device> --apps -`,
Copy link
Member

Choose a reason for hiding this comment

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

explain what "parent" and "none" mean here also

Copy link
Member

Choose a reason for hiding this comment

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

I would state explicitly here that we are inheriting from the group config.

Suggested change
fioctl devices config updates <device> --apps -`,
# Set the docker apps to inherit from a group config:

We might be more verbose (like below), but I think that is apparent from the fioctl config updates help.

Suggested change
fioctl devices config updates <device> --apps -`,
# Set the docker apps to inherit from a group config (if set there) or a system default (otherwise):

Copy link
Member

Choose a reason for hiding this comment

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

Once I wrote this I understood we don't yet have a way to restore the device tag to inherit from a group config or system default.
Do we need one?

@doanac @mike-scott ^^^

Copy link
Member

Choose a reason for hiding this comment

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

probably should do it also

}
configCmd.AddCommand(configUpdatesCmd)
configUpdatesCmd.Flags().StringP("tag", "", "", "Target tag for device to follow")
Expand Down
Loading