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

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

Merged

Conversation

farshidtz
Copy link
Member

@farshidtz farshidtz commented Oct 6, 2022

This refactoring is to make use of the new snapctl and log packages from the same module.

If your build fails due to your commit message not passing the build checks, please review the guidelines here: https://github.com/edgexfoundry/edgex-go/blob/main/.github/Contributing.md

PR Checklist

Please check if your PR fulfills the following requirements:

  • I am not introducing a breaking change (if you are, flag in conventional commit message with BREAKING CHANGE: describing the break)
  • I am not introducing a new dependency (add notes below if you are)
  • I have added unit tests for the new feature or bug fix (if not, why?)
  • I have fully tested (add details below) this the new feature or bug fix (if not, why?)
  • I have opened a PR for the related docs change (if not, why?)

Testing Instructions

New Dependency Instructions (If applicable)

@farshidtz farshidtz changed the title Snap use snapctl log packages refactor(snap): Use snapctl and log packages Oct 6, 2022
@farshidtz farshidtz requested a review from MonicaisHer October 6, 2022 16:25
@farshidtz
Copy link
Member Author

This builds on top of #4186 and needs to be rebased once that is merged. The review should only consider the new commit: c4e9203

Signed-off-by: Farshid Tavakolizadeh <farshid.tavakolizadeh@canonical.com>
@farshidtz farshidtz force-pushed the snap-use-snapctl-log-packages branch from c4e9203 to 57ab585 Compare October 7, 2022 10:33
@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 7, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
4.1% 4.1% Duplication

@farshidtz farshidtz marked this pull request as ready for review October 7, 2022 13:28
Copy link
Contributor

@MonicaisHer MonicaisHer left a comment

Choose a reason for hiding this comment

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

@farshidtz Thank you for the lots of changes! It works as expected. I have some minor questions please see the inline comments. And here is what I tested:

snap install edgexfoundry_2.3.0-dev.83_amd64.snap --dangerous

sudo snap set edgexfoundry env.core-data.service.port=1111
snap set edgexfoundry.kuiper=on

snap set edgexfoundry app-options=true
snap set edgexfoundry apps.core-data.config.service-port=2222
snap restart edgexfoundry.core-data

# install hook logs
Okt 10 13:58:33 ubuntu systemd[1]: Started snap.edgexfoundry.hook.install.f2799bcf-e72b-43c9-a7f8-04d5c9b29125.scope.
Okt 10 13:58:39 ubuntu edgexfoundry.install[1209658]: Disabling all services: [edgexfoundry.consul edgexfoundry.core-command edgexfoundry.kuiper edgexfoundry.redis edgexfoundry.vault edgexfoundry.security-consul-bootstrapper edgexfoundry.security-secretstore-setup edgexfoundry.app-service-configurable edgexfoundry.postgres edgexfoundry.support-notifications edgexfoundry.support-scheduler edgexfoundry.sys-mgmt-agent edgexfoundry.core-data edgexfoundry.core-metadata edgexfoundry.kong-daemon edgexfoundry.security-bootstrapper-redis edgexfoundry.security-proxy-setup]
Okt 10 13:58:39 ubuntu systemd[1]: snap.edgexfoundry.hook.install.f2799bcf-e72b-43c9-a7f8-04d5c9b29125.scope: Succeeded.
Okt 10 13:58:39 ubuntu edgexfoundry.configure[1209935]: install-mode=defer-startup
Okt 10 13:58:39 ubuntu edgexfoundry.configure[1209935]: install-mode=defer-startup; starting disabled services

# configure hook logs
Okt 10 14:05:04 ubuntu systemd[1]: Started snap.edgexfoundry.hook.configure.51f81f20-e239-4438-9fe1-635f9ea4acac.scope.
Okt 10 14:05:04 ubuntu edgexfoundry.configure[1215288]: Processing config options
Okt 10 14:05:04 ubuntu edgexfoundry.configure[1215288]: Processing app options: false
Okt 10 14:05:04 ubuntu edgexfoundry.configure[1215288]: Started
Okt 10 14:05:04 ubuntu edgexfoundry.configure[1215288]: install-mode=defer-startup
Okt 10 14:05:04 ubuntu edgexfoundry.configure[1215288]: install-mode=defer-startup; starting disabled services
Okt 10 14:05:04 ubuntu systemd[1]: snap.edgexfoundry.hook.configure.51f81f20-e239-4438-9fe1-635f9ea4acac.scope: Succeeded.
Okt 10 14:05:30 ubuntu edgexfoundry.security-consul-bootstrapper[1216021]: level=INFO ts=2022-10-10T12:05:30.613824293Z app=security-bootstrapper source=command.go:699 msg="successfully configure Consul access for secretstore"
Okt 10 14:05:32 ubuntu systemd[1]: Started snap.edgexfoundry.hook.configure.32d12c39-5d90-48cd-81d5-95dd65fc4089.scope.
Okt 10 14:05:32 ubuntu edgexfoundry.configure[1216360]: Processing config options
Okt 10 14:05:32 ubuntu edgexfoundry.configure[1216360]: Processing app options: false
Okt 10 14:05:32 ubuntu edgexfoundry.configure[1216360]: Started
Okt 10 14:05:32 ubuntu edgexfoundry.configure[1216360]: install-mode=
Okt 10 14:05:32 ubuntu systemd[1]: snap.edgexfoundry.hook.configure.32d12c39-5d90-48cd-81d5-95dd65fc4089.scope: Succeeded.
Okt 10 14:05:32 ubuntu systemd[1]: Started snap.edgexfoundry.hook.configure.e61a057b-c5a9-4dda-8322-e93be18d2a3e.scope.
Okt 10 14:05:32 ubuntu edgexfoundry.configure[1216620]: Processing config options
Okt 10 14:05:32 ubuntu edgexfoundry.configure[1216620]: Processing app options: true
Okt 10 14:05:32 ubuntu edgexfoundry.configure[1216620]: Unset all 'env.' options.
Okt 10 14:05:32 ubuntu edgexfoundry.configure[1216620]: Mapping add-known-secrets to ADD_KNOWN_SECRETS
Okt 10 14:05:32 ubuntu edgexfoundry.configure[1216620]: Mapping add-secretstore-tokens to ADD_SECRETSTORE_TOKENS
Okt 10 14:05:32 ubuntu edgexfoundry.configure[1216620]: Mapping add-registry-acl-roles to ADD_REGISTRY_ACL_ROLES
Okt 10 14:05:32 ubuntu edgexfoundry.configure[1216620]: Writing to env file /var/snap/edgexfoundry/x1/config/security-secretstore-setup/res/security-secretstore-setup.env: ADD_KNOWN_SECRETS="redisdb[app-functional-tests],redisdb[app-rules-engine],redisdb[app-http-export],redisdb[app-mqtt-export],redisdb[app-external-mqtt-trigger],redisdb[app-push-to-core],redisdb[app-rfid-llrp-inventory],redisdb[application-service],redisdb[device-rest],redisdb[device-virtual],redisdb[device-camera],redisdb[device-mqtt],redisdb[device-modbus],redisdb[device-coap],redisdb[device-snmp],redisdb[device-gpio],redisdb[device-bacnet],redisdb[device-grove],redisdb[device-uart],redisdb[device-rfid-llrp],redisdb[device-usb-camera],redisdb[device-onvif-camera],redisdb[edgex-ekuiper]" ADD_SECRETSTORE_TOKENS="app-functional-tests,app-rules-engine,app-http-export,app-mqtt-export,app-external-mqtt-trigger,app-push-to-core,app-rfid-llrp-inventory,application-service,device-camera,device-mqtt,device-modbus,device-coap,device-snmp,device-gpio,device-bacnet,device-grove,device-uart,device-rfid-llrp,device-usb-camera,device-onvif-camera,edgex-ekuiper"
Okt 10 14:05:32 ubuntu edgexfoundry.configure[1216620]: Writing to env file /var/snap/edgexfoundry/x1/config/security-bootstrapper/res/security-bootstrapper.env: ADD_REGISTRY_ACL_ROLES="app-functional-tests,app-rules-engine,app-http-export,app-mqtt-export,app-external-mqtt-trigger,app-push-to-core,app-rfid-llrp-inventory,application-service,device-camera,device-mqtt,device-modbus,device-coap,device-snmp,device-gpio,device-bacnet,device-grove,device-uart,device-rfid-llrp,device-usb-camera,device-onvif-camera,edgex-ekuiper"
Okt 10 14:05:32 ubuntu edgexfoundry.configure[1216620]: Started
Okt 10 14:05:32 ubuntu edgexfoundry.configure[1216620]: install-mode=
Okt 10 14:05:32 ubuntu systemd[1]: snap.edgexfoundry.hook.configure.e61a057b-c5a9-4dda-8322-e93be18d2a3e.scope: Succeeded.

# options logs
Okt 10 14:05:32 ubuntu edgexfoundry.options[1216162]: Configuring options for security-proxy
Okt 10 14:05:32 ubuntu edgexfoundry.options[1216183]: Configuring options for secrets-config

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.


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

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)

snap/local/helper-go/configure.go Show resolved Hide resolved
@farshidtz farshidtz merged commit d47f91d into edgexfoundry:main Oct 11, 2022
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.

2 participants