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

refactor(snap): Use snapctl and log packages #4187

Merged
merged 1 commit into from
Oct 11, 2022
Merged

refactor(snap): Use snapctl and log packages #4187

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
141 changes: 63 additions & 78 deletions snap/local/helper-go/configure.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ import (
"strings"

hooks "github.com/canonical/edgex-snap-hooks/v2"
"github.com/canonical/edgex-snap-hooks/v2/log"
"github.com/canonical/edgex-snap-hooks/v2/snapctl"
)

const ( // iota is reset to 0
Expand Down Expand Up @@ -99,7 +101,7 @@ func getOptServices() []string {
func isDisableAllowed(s string) error {
for _, v := range getRequiredServices() {
if s == v {
return fmt.Errorf("edgexfoundry:configure: can't disable required service: %s", s)
return fmt.Errorf("can't disable required service: %s", s)
}
}
return nil
Expand All @@ -112,23 +114,23 @@ func handleSingleService(name, state string) error {

switch state {
case OFF:
hooks.Debug("edgexfoundry:configure: state: off")
if err := cli.Stop(name, true); err != nil {
log.Debugf("%s state: off", name)
if err := snapctl.Stop(hooks.SnapName + "." + name).Disable().Run(); err != nil {
return err
}
if err := cli.SetConfig(name, OFF); err != nil {
if err := snapctl.Set(name, OFF).Run(); err != nil {
return err
}
case ON:
hooks.Debug("edgexfoundry:configure: state: on")
if err := cli.Start(name, true); err != nil {
log.Debugf("%s state: on", name)
if err := snapctl.Start(hooks.SnapName + "." + name).Enable().Run(); err != nil {
return err
}
if err := cli.SetConfig(name, ON); err != nil {
if err := snapctl.Set(name, ON).Run(); err != nil {
return err
}
default:
return fmt.Errorf("edgexfoundry:configure: invalid state %s for service: %s", state, name)
return fmt.Errorf("invalid state %s for service: %s", state, name)
}

return nil
Expand Down Expand Up @@ -194,12 +196,12 @@ func rmConsulAclFile() error {
}

func disableSecretStoreAndRestart() error {
hooks.Info(fmt.Sprintf("edgexfoundry:configure: disabling secret store"))
log.Info("disabling secret store")

// if consul_acls.json doesn't exist, then secret-store has already been
// disabled, so just return
if !consulAclFileExists() {
hooks.Info(fmt.Sprintf("edgexfoundry:configure: secret store is already disabled"))
log.Info("secret store is already disabled")
return nil
}

Expand All @@ -222,7 +224,7 @@ func disableSecretStoreAndRestart() error {
// snap config option for each service to be needlessly set to "off"
// then back to "on"; re-factor handleServices/handleSingleService
for _, s := range getEdgeXRefServices() {
if err := cli.Stop(s, false); err != nil {
if err := snapctl.Stop(hooks.SnapName + "." + s).Run(); err != nil {
return err
}
}
Expand All @@ -231,18 +233,18 @@ func disableSecretStoreAndRestart() error {
// TODO - kuiper will be stopped, but not restarted because
// additional re-configuration may be needed.
for _, s := range getKuiperServices() {
if err := cli.Stop(s, false); err != nil {
if err := snapctl.Stop(hooks.SnapName + "." + s).Run(); err != nil {
return err
}
}

// stop redis
if err := cli.Stop("redis", false); err != nil {
if err := snapctl.Stop(hooks.SnapName + "." + "redis").Run(); err != nil {
return err
}

// stop consul
if err := cli.Stop("consul", false); err != nil {
if err := snapctl.Stop(hooks.SnapName + "." + "consul").Run(); err != nil {
return err
}

Expand All @@ -258,15 +260,15 @@ func disableSecretStoreAndRestart() error {

// - start required services
for _, s := range getRequiredServices() {
if err := cli.Start(s, false); err != nil {
if err := snapctl.Start(hooks.SnapName + "." + s).Run(); err != nil {
return err
}
}

// Now check config status of the optional EdgeX
// services and restart where necessary
for _, s := range getEdgeXRefServices() {
status, err := cli.Config(s)
status, err := snapctl.Get(s).Run()
if err != nil {
return err
}
Expand All @@ -277,7 +279,7 @@ func disableSecretStoreAndRestart() error {
// part of the enabledServices (i.e. services
// always started), then also start it
if status == ON || (status == "" && strings.HasPrefix(s, "core-")) {
if err := cli.Start(s, false); err != nil {
if err := snapctl.Start(hooks.SnapName + "." + s).Run(); err != nil {
return err
}
}
Expand Down Expand Up @@ -310,12 +312,12 @@ func handleAllServices(deferStartup bool) error {
for _, s := range hooks.Services {
var serviceList []string

status, err := cli.Config(s)
status, err := snapctl.Get(s).Run()
if err != nil {
return err
}

hooks.Debug(fmt.Sprintf("edgexfoundry:configure: service: %s status: %s", s, status))
log.Debugf("service: %s status: %s", s, status)

err = applyConfigOptions(s)
if err != nil {
Expand All @@ -339,7 +341,7 @@ func handleAllServices(deferStartup bool) error {

switch sType {
case kuiperService:
hooks.Debug("edgexfoundry:configure: kuiper")
log.Debug("kuiper")

switch status {
case ON, OFF:
Expand All @@ -349,11 +351,11 @@ func handleAllServices(deferStartup bool) error {
// configuration has been specified; no-op
continue
default:
return fmt.Errorf("edgexfoundry:configure: invalid value for kuiper: %s", status)
return fmt.Errorf("invalid value for kuiper: %s", status)
}

case secProxyService:
hooks.Debug("edgexfoundry:configure: proxy")
log.Debug("proxy")

switch status {
case ON:
Expand All @@ -363,9 +365,7 @@ func handleAllServices(deferStartup bool) error {
// does not automatically handle enabling the secret-store
// if/when the proxy is dynamically enabled.
if !secretStoreActive {
err = fmt.Errorf("edgexfoundry:configure security-proxy=on not allowed;" +
"secret-store=off")
return err
return fmt.Errorf("security-proxy=on not allowed when secret-store is off")
}

fallthrough
Expand All @@ -376,15 +376,15 @@ func handleAllServices(deferStartup bool) error {
// configuration has been specified; no-op
continue
default:
return fmt.Errorf("edgexfoundry:configure: invalid value for security-proxy: %s", status)
return fmt.Errorf("invalid value for security-proxy: %s", status)
}

case secStoreService:
hooks.Debug("edgexfoundry:configure: secretstore")
log.Debug("secretstore")

switch status {
case ON:
return fmt.Errorf("edgexfoundry:configure security-secret-store=on not allowed")
return fmt.Errorf("security-secret-store=on not allowed")
case OFF:
// TODO - this var is used by the secProxyCase to ensure that the
// secret store is active when the proxy is being enabled at runtime.
Expand All @@ -402,11 +402,11 @@ func handleAllServices(deferStartup bool) error {
// configuration has been specified; no-op
continue
default:
return fmt.Errorf("edgexfoundry:configure: invalid value for security-secret-store: %s", status)
return fmt.Errorf("invalid value for security-secret-store: %s", status)
}

default:
hooks.Debug("edgexfoundry:configure: other service")
log.Debugf("other service: %s", s)
// default case for all other services

switch status {
Expand All @@ -422,11 +422,11 @@ func handleAllServices(deferStartup bool) error {
// configuration has been specified; no-op
continue
default:
return fmt.Errorf("edgexfoundry:configure: invalid value for %s: %s", s, status)
return fmt.Errorf("invalid value for %s: %s", s, status)
}
}

hooks.Debug(fmt.Sprintf("edgexfoundry:configure calling handleServices: %v", serviceList))
log.Debugf("calling handleServices: %v", serviceList)
if err = handleServices(serviceList, status); err != nil {
return err
}
Expand All @@ -438,7 +438,7 @@ func handleAllServices(deferStartup bool) error {
func checkCoreConfig(services []string) ([]string, error) {
// walk thru the list of default services
for _, s := range getCoreDefaultServices() {
status, err := cli.Config(s)
status, err := snapctl.Get(s).Run()
if err != nil {
return nil, err
}
Expand All @@ -449,7 +449,7 @@ func checkCoreConfig(services []string) ([]string, error) {
case ON, UNSET:
services = append(services, s)
default:
err = fmt.Errorf("edgexfoundry:configure: invalid value: %s for %s", status, s)
err = fmt.Errorf("invalid value: %s for %s", status, s)
return nil, err
}
}
Expand All @@ -459,7 +459,7 @@ func checkCoreConfig(services []string) ([]string, error) {
func checkOptConfig(services []string) ([]string, error) {
// walk thru the list of default services
for _, s := range getOptServices() {
status, err := cli.Config(s)
status, err := snapctl.Get(s).Run()
if err != nil {
return nil, err
}
Expand All @@ -470,7 +470,7 @@ func checkOptConfig(services []string) ([]string, error) {
case ON:
services = append(services, s)
default:
err = fmt.Errorf("edgexfoundry:configure: invalid value: %s for %s", status, s)
err = fmt.Errorf("invalid value: %s for %s", status, s)
return nil, err
}
}
Expand All @@ -479,7 +479,7 @@ func checkOptConfig(services []string) ([]string, error) {

func checkSecurityConfig(services []string) ([]string, error) {

status, err := cli.Config("security-secret-store")
status, err := snapctl.Get("security-secret-store").Run()
if err != nil {
return nil, err
}
Expand All @@ -492,12 +492,12 @@ func checkSecurityConfig(services []string) ([]string, error) {
// default behavior
services = append(services, getSecretStoreServices()...)
default:
err = fmt.Errorf("edgexfoundry:configure: invalid setting for security-secret-store: %s", status)
err = fmt.Errorf("invalid setting for security-secret-store: %s", status)
return nil, err
}

// check secret-proxy
status, err = cli.Config("security-proxy")
status, err = snapctl.Get("security-proxy").Run()
if err != nil {
return nil, err
}
Expand All @@ -509,48 +509,35 @@ func checkSecurityConfig(services []string) ([]string, error) {
// default behavior
services = append(services, getProxyServices()...)
default:
err = fmt.Errorf("edgexfoundry:configure: invalid setting for security-proxy: %s", status)
err = fmt.Errorf("invalid setting for security-proxy: %s", status)
return nil, err
}
return services, nil
}

func configure() {
log.SetComponentName("configure")

// process the EdgeX >=2.2 app options
processAppOptions()
if err := processAppOptions(); err != nil {
log.Fatalf("error processing app options: %v", err)
}

var debug = false
var err error
var startServices []string

status, err := cli.Config("debug")
if err != nil {
fmt.Println(fmt.Sprintf("edgexfoundry:configure: can't read value of 'debug': %v", err))
os.Exit(1)
}
if status == "true" {
debug = true
}

if err = hooks.Init(debug, "edgexfoundry"); err != nil {
fmt.Println(fmt.Sprintf("edgexfoundry:configure: initialization failure: %v", err))
os.Exit(1)
}
hooks.Info("edgexfoundry:configure: started")
log.Info("Started")
Copy link
Contributor

Choose a reason for hiding this comment

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

How about moving this info to line 520, so it will be ahead of processing app options?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have addressed this and your next comment in my upcoming refactoring, which will be added once this PR is merged. See https://github.com/farshidtz/edgex-go/blob/cbd7714b276f42495f659f666f7270833e296979/snap/local/helper-go/configure.go#L544-L551

If you agree, I'd like to keep it that way to avoid merge conflicts.


val, err := cli.Config("install-mode")
installMode, err := snapctl.Get("install-mode").Run()
if err != nil {
hooks.Error(fmt.Sprintf("edgexfoundry:configure: reading 'install-mode': %v", err))
os.Exit(1)
log.Fatalf("error reading 'install-mode': %v", err)
}

deferStartup := (val == "defer-startup")
hooks.Info(fmt.Sprintf("edgexfoundry:configure: deferStartup=%v", deferStartup))
log.Info("install-mode=%s", installMode)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
log.Info("install-mode=%s", installMode)
log.Infof(`install-mode="%s"`, installMode)

It would be clear if install-mode is unset

Copy link
Member Author

@farshidtz farshidtz Oct 11, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I agree and I will address it in my next PR. See my previous comment: #4187 (comment)

deferStartup := (installMode == "defer-startup")

// handle per service configuration and enable/disable services
if err = handleAllServices(deferStartup); err != nil {
hooks.Error(fmt.Sprintf("edgexfoundry:configure: error handling services: %v", err))
os.Exit(1)
log.Fatalf("error handling services: %v", err)
}

// Handle deferred startup of services disabled in the install hook.
Expand All @@ -562,16 +549,15 @@ func configure() {
// before the config hook runs), leaving the duplication means less
// re-factoring if/when snapd adds a new hook.
if deferStartup {
hooks.Info(fmt.Sprintf("edgexfoundry:configure install-mode=defer-startup; starting disabled services"))
log.Info("install-mode=defer-startup; starting disabled services")

// add required services
startServices = append(startServices, getRequiredServices()...)

// check security configuration
startServices, err = checkSecurityConfig(startServices)
if err != nil {
hooks.Error(fmt.Sprintf("edgexfoundry:configure: security config error; %v", err))
os.Exit(1)
log.Fatalf("security service config error: %v", err)
}

// TODO: don't support kuiper until it's possible to share
Expand All @@ -581,26 +567,25 @@ func configure() {
// check core services
startServices, err = checkCoreConfig(startServices)
if err != nil {
hooks.Error(fmt.Sprintf("edgexfoundry:configure: core config error; %v", err))
os.Exit(1)
log.Fatalf("core service config error: %v", err)
}

// check optional services
startServices, err = checkOptConfig(startServices)
if err != nil {
hooks.Error(fmt.Sprintf("edgexfoundry:configure: optional config error; %v", err))
os.Exit(1)
log.Fatalf("optional service config error: %v", err)
}

// NOTE - the services will be started after the configure hook finishes
if err = cli.StartMultiple(true, startServices...); err != nil {
hooks.Error(fmt.Sprintf("edgexfoundry:configure failure starting/enabling services: %v", err))
os.Exit(1)
for i, s := range startServices {
startServices[i] = hooks.SnapName + "." + s
farshidtz marked this conversation as resolved.
Show resolved Hide resolved
}
// NOTE: the services will be scheduled to start by snapd after the configure hook exits
if err = snapctl.Start(startServices...).Enable().Run(); err != nil {
log.Fatalf("error starting/enabling services: %v", err)
}

if err = cli.UnsetConfig("install-mode"); err != nil {
hooks.Error(fmt.Sprintf("edgexfoundry:install un-setting 'install'; %v", err))
os.Exit(1)
if err = snapctl.Unset("install-mode").Run(); err != nil {
log.Fatalf("error un-setting 'install'; %v", err)
}
}
}
Loading