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

[APP-7692] Fix empty config errors on new install #73

Merged
merged 11 commits into from
Feb 24, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions .github/workflows/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
fetch-depth: 0
- uses: actions/setup-go@v5
with:
go-version-file: go.mod
Expand Down
5 changes: 4 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,11 @@ else ifeq ($(GOARCH),arm64)
LINUX_ARCH = aarch64
endif

GITHUB_REF_NAME ?= $(shell git branch --show-current)
SHOULD_PUBLISH = $(shell echo $(GITHUB_REF_NAME) | grep -qE '^(main|v[0-9]+\.[0-9]+\.[0-9]+)$$' && echo true)

ifeq ($(shell git status -s),)
ifeq ($(shell git branch --show-current),main)
ifeq ($(SHOULD_PUBLISH),true)
LAST_TAG := $(shell git describe --tags --abbrev=0 2>/dev/null)
COMMITS_SINCE_TAG := $(shell git rev-list $(LAST_TAG)..HEAD --count 2>/dev/null)
BASE_VERSION := $(shell echo $(LAST_TAG) | cut -c2-)
Expand Down
4 changes: 3 additions & 1 deletion cmd/viam-agent/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,9 @@ func main() {
globalLogger.Infof("machine credentials file path: %s", utils.AppConfigFilePath)

cfg, err := utils.LoadConfigFromCache()
exitIfError(err)
if err != nil {
globalLogger.Errorf("error loading config from cache: %w", err)
}
Comment on lines +134 to +136
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 should be non-fatal always now.


cfg = utils.ApplyCLIArgs(cfg)

Expand Down
6 changes: 3 additions & 3 deletions subsystems/networking/grpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ func (n *Networking) GetSmartMachineStatus(ctx context.Context,

ret := &pb.GetSmartMachineStatusResponse{
ProvisioningInfo: &pb.ProvisioningInfo{
FragmentId: n.cfg.FragmentID,
Model: n.cfg.Model,
Manufacturer: n.cfg.Manufacturer,
FragmentId: n.Config().FragmentID,
Model: n.Config().Model,
Manufacturer: n.Config().Manufacturer,
Comment on lines +42 to +44
Copy link
Member Author

Choose a reason for hiding this comment

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

There's one fix in config.go, the rest is a related fix around config locking (potential race.) Overlooked during 0.14.0 development.

},
HasSmartMachineCredentials: n.connState.getConfigured(),
IsOnline: n.connState.getOnline(),
Expand Down
26 changes: 16 additions & 10 deletions subsystems/networking/networking.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ func (n *Networking) init(ctx context.Context) error {
n.nm = nm
n.settings = settings

n.netState.SetHotspotInterface(n.cfg.HotspotInterface)
n.netState.SetHotspotInterface(n.Config().HotspotInterface)

if err := n.writeDNSMasq(); err != nil {
return errw.Wrap(err, "writing dnsmasq configuration")
Expand Down Expand Up @@ -171,7 +171,7 @@ func (n *Networking) init(ctx context.Context) error {

n.warnIfMultiplePrimaryNetworks()

if n.cfg.TurnOnHotspotIfWifiHasNoInternet {
if n.Config().TurnOnHotspotIfWifiHasNoInternet {
n.logger.Info("Wifi internet checking enabled. Will try all connections for global internet connectivity.")
} else {
primarySSID := n.netState.PrimarySSID(n.Config().HotspotInterface)
Expand Down Expand Up @@ -277,12 +277,12 @@ func (n *Networking) Update(ctx context.Context, cfg utils.AgentConfig) (needRes
}
n.netState.SetHotspotInterface(cfg.NetworkConfiguration.HotspotInterface)

if reflect.DeepEqual(cfg.NetworkConfiguration, n.cfg) && reflect.DeepEqual(cfg.AdditionalNetworks, n.nets) {
if reflect.DeepEqual(cfg.NetworkConfiguration, n.Config()) && reflect.DeepEqual(cfg.AdditionalNetworks, n.nets) {
return needRestart
}

needRestart = true
n.logger.Debugf("Updated config differs from previous. Previous: %+v New: %+v", n.cfg, cfg)
n.logger.Debugf("Updated config differs from previous. Previous: %#v New: %#v", n.Config(), cfg)

n.dataMu.Lock()
defer n.dataMu.Unlock()
Expand Down Expand Up @@ -317,12 +317,18 @@ func (n *Networking) Config() utils.NetworkConfiguration {
return n.cfg
}

func (n *Networking) Nets() utils.AdditionalNetworks {
n.dataMu.Lock()
defer n.dataMu.Unlock()
return n.nets
}

func (n *Networking) processAdditionalnetworks(ctx context.Context) {
if !n.cfg.TurnOnHotspotIfWifiHasNoInternet && len(n.nets) > 0 {
if !n.Config().TurnOnHotspotIfWifiHasNoInternet && len(n.Nets()) > 0 {
n.logger.Warn("Additional networks configured, but internet checking is not enabled. Additional networks may be unused.")
}

for _, network := range n.nets {
for _, network := range n.Nets() {
_, err := n.addOrUpdateConnection(network)
if err != nil {
n.logger.Error(errw.Wrapf(err, "adding network %s", network.SSID))
Expand All @@ -339,8 +345,8 @@ func (n *Networking) processAdditionalnetworks(ctx context.Context) {
// must be run inside dataMu lock.
func (n *Networking) writeWifiPowerSave(ctx context.Context) error {
contents := wifiPowerSaveContentsDefault
if n.cfg.WifiPowerSave != nil {
if *n.cfg.WifiPowerSave {
if n.Config().WifiPowerSave != nil {
if *n.Config().WifiPowerSave {
contents = wifiPowerSaveContentsEnable
} else {
contents = wifiPowerSaveContentsDisable
Expand All @@ -359,9 +365,9 @@ func (n *Networking) writeWifiPowerSave(ctx context.Context) error {
return errw.Wrap(err, "reloading NetworkManager after wifi-powersave.conf update")
}

ssid := n.netState.ActiveSSID(n.cfg.HotspotInterface)
ssid := n.netState.ActiveSSID(n.Config().HotspotInterface)
if n.connState.getConnected() && ssid != "" {
if err := n.activateConnection(ctx, n.cfg.HotspotInterface, ssid); err != nil {
if err := n.activateConnection(ctx, n.Config().HotspotInterface, ssid); err != nil {
return errw.Wrapf(err, "reactivating %s to enforce powersave setting", ssid)
}
}
Expand Down
53 changes: 29 additions & 24 deletions subsystems/networking/networkmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
)

func (n *Networking) warnIfMultiplePrimaryNetworks() {
if n.cfg.TurnOnHotspotIfWifiHasNoInternet {
if n.Config().TurnOnHotspotIfWifiHasNoInternet {
return
}
var primaryCandidates []string
Expand Down Expand Up @@ -149,12 +149,12 @@ func (n *Networking) checkConnections() error {
}

// in roaming mode, we don't care WHAT network is connected
if n.cfg.TurnOnHotspotIfWifiHasNoInternet && state == gnm.NmActiveConnectionStateActivated && ssid != n.Config().HotspotSSID {
if n.Config().TurnOnHotspotIfWifiHasNoInternet && state == gnm.NmActiveConnectionStateActivated && ssid != n.Config().HotspotSSID {
connected = true
}

// in normal (single) mode, we need to be connected to the primary (highest priority) network
if !n.cfg.TurnOnHotspotIfWifiHasNoInternet && state == gnm.NmActiveConnectionStateActivated &&
if !n.Config().TurnOnHotspotIfWifiHasNoInternet && state == gnm.NmActiveConnectionStateActivated &&
ssid == n.netState.PrimarySSID(n.Config().HotspotInterface) {
connected = true
}
Expand Down Expand Up @@ -273,7 +273,7 @@ func (n *Networking) activateConnection(ctx context.Context, ifName, ssid string

if nw.netType != NetworkTypeHotspot {
n.netState.SetActiveSSID(ifName, ssid)
if ifName == n.Config().HotspotInterface && (n.cfg.TurnOnHotspotIfWifiHasNoInternet || n.netState.PrimarySSID(ifName) == ssid) {
if ifName == n.Config().HotspotInterface && (n.Config().TurnOnHotspotIfWifiHasNoInternet || n.netState.PrimarySSID(ifName) == ssid) {
n.connState.setConnected(true)
}
return n.checkOnline(true)
Expand Down Expand Up @@ -389,17 +389,22 @@ func (n *Networking) addOrUpdateConnection(cfg utils.NetworkDefinition) (bool, e
return changesMade, errw.Errorf("only the builtin provisioning hotspot may use the %s network type", NetworkTypeHotspot)
}
nw.isHotspot = true
settings = generateHotspotSettings(n.cfg.HotspotPrefix, n.Config().HotspotSSID, n.cfg.HotspotPassword, n.Config().HotspotInterface)
settings = generateHotspotSettings(
n.Config().HotspotPrefix,
n.Config().HotspotSSID,
n.Config().HotspotPassword,
n.Config().HotspotInterface,
)
} else {
id := n.cfg.Manufacturer + "-" + netKey
id := n.Config().Manufacturer + "-" + netKey
settings, err = generateNetworkSettings(id, cfg)
n.logger.Debugf("Network settings: ", settings)
n.logger.Debugf("Network settings: %#v", settings)
if err != nil {
return changesMade, errw.Errorf("error generating network settings for %s: %v", id, err)
}
}

if cfg.Type == NetworkTypeWifi && !n.cfg.TurnOnHotspotIfWifiHasNoInternet && cfg.Priority == 999 {
if cfg.Type == NetworkTypeWifi && !n.Config().TurnOnHotspotIfWifiHasNoInternet && cfg.Priority == 999 {
// lower the priority of any existing/prior primary network
n.lowerMaxNetPriorities(cfg.SSID)
n.netState.SetPrimarySSID(n.Config().HotspotInterface, cfg.SSID)
Expand Down Expand Up @@ -495,7 +500,7 @@ func (n *Networking) tryCandidates(ctx context.Context) bool {
}

// in single mode we just need a connection
if !n.cfg.TurnOnHotspotIfWifiHasNoInternet {
if !n.Config().TurnOnHotspotIfWifiHasNoInternet {
return true
}

Expand Down Expand Up @@ -523,14 +528,14 @@ func (n *Networking) getCandidates(ifName string) []string {

// firstSeen/lastTried are reset if a network disappears for more than a minute, so retry if it comes back (or 10 mins)
recentlyTried := nw.lastTried.After(nw.firstSeen) &&
nw.lastTried.After(time.Now().Add(time.Duration(n.cfg.RetryConnectionTimeoutMinutes)*-1))
nw.lastTried.After(time.Now().Add(time.Duration(n.Config().RetryConnectionTimeoutMinutes)*-1))

if !nw.isHotspot && visible && configured && !recentlyTried {
candidates = append(candidates, nw)
}
}

if !n.cfg.TurnOnHotspotIfWifiHasNoInternet {
if !n.Config().TurnOnHotspotIfWifiHasNoInternet {
for _, nw := range candidates {
if nw.ssid == n.netState.PrimarySSID(n.Config().HotspotInterface) {
return []string{nw.ssid}
Expand Down Expand Up @@ -608,7 +613,7 @@ func (n *Networking) mainLoop(ctx context.Context) {
if userInput.SSID != "" {
n.logger.Infof("Wifi settings received for %s", userInput.SSID)
priority := int32(999)
if n.cfg.TurnOnHotspotIfWifiHasNoInternet {
if n.Config().TurnOnHotspotIfWifiHasNoInternet {
priority = 100
}
cfg := utils.NetworkDefinition{
Expand Down Expand Up @@ -684,7 +689,7 @@ func (n *Networking) mainLoop(ctx context.Context) {
}
isConfigured := n.connState.getConfigured()
allGood := isConfigured && (isConnected || isOnline)
if n.cfg.TurnOnHotspotIfWifiHasNoInternet {
if n.Config().TurnOnHotspotIfWifiHasNoInternet {
allGood = isOnline && isConfigured
hasConnectivity = isOnline
lastConnectivity = lastOnline
Expand All @@ -704,16 +709,16 @@ func (n *Networking) mainLoop(ctx context.Context) {
// complex logic, so wasting some variables for readability

// portal interaction time is updated when a user loads a page or makes a grpc request
inactivePortal := n.connState.getLastInteraction().Before(now.Add(time.Duration(n.cfg.UserIdleMinutes)*-1)) || userInputReceived
inactivePortal := n.connState.getLastInteraction().Before(now.Add(time.Duration(n.Config().UserIdleMinutes)*-1)) || userInputReceived

// exit/retry to test networks only if there's no recent user interaction AND configuration is present
haveCandidates := len(n.getCandidates(n.Config().HotspotInterface)) > 0 && inactivePortal && isConfigured

// exit/retry every FallbackTimeout (10 minute default), unless user is active
fallbackHit := pModeChange.Before(now.Add(time.Duration(n.cfg.RetryConnectionTimeoutMinutes)*-1)) && inactivePortal
fallbackHit := pModeChange.Before(now.Add(time.Duration(n.Config().RetryConnectionTimeoutMinutes)*-1)) && inactivePortal

shouldReboot := n.cfg.DeviceRebootAfterOfflineMinutes > 0 &&
lastConnectivity.Before(now.Add(time.Duration(n.cfg.DeviceRebootAfterOfflineMinutes)*-1))
shouldReboot := n.Config().DeviceRebootAfterOfflineMinutes > 0 &&
lastConnectivity.Before(now.Add(time.Duration(n.Config().DeviceRebootAfterOfflineMinutes)*-1))

shouldExit := allGood || haveCandidates || fallbackHit || shouldReboot

Expand Down Expand Up @@ -742,28 +747,28 @@ func (n *Networking) mainLoop(ctx context.Context) {
if n.tryCandidates(ctx) {
hasConnectivity = n.connState.getConnected() || n.connState.getOnline()
// if we're roaming or this network was JUST added, it must have internet
if n.cfg.TurnOnHotspotIfWifiHasNoInternet {
if n.Config().TurnOnHotspotIfWifiHasNoInternet {
hasConnectivity = n.connState.getOnline()
}
if hasConnectivity {
continue
}
lastConnectivity = n.connState.getLastConnected()
if n.cfg.TurnOnHotspotIfWifiHasNoInternet {
if n.Config().TurnOnHotspotIfWifiHasNoInternet {
lastConnectivity = n.connState.getLastOnline()
}
}
}

shouldReboot := n.cfg.DeviceRebootAfterOfflineMinutes > 0 &&
lastConnectivity.Before(now.Add(time.Duration(n.cfg.DeviceRebootAfterOfflineMinutes)*-1))
shouldReboot := n.Config().DeviceRebootAfterOfflineMinutes > 0 &&
lastConnectivity.Before(now.Add(time.Duration(n.Config().DeviceRebootAfterOfflineMinutes)*-1))

if shouldReboot && n.doReboot(ctx) {
return
}

hitOfflineTimeout := lastConnectivity.Before(now.Add(time.Duration(n.cfg.OfflineBeforeStartingHotspotMinutes)*-1)) &&
pModeChange.Before(now.Add(time.Duration(n.cfg.OfflineBeforeStartingHotspotMinutes)*-1))
hitOfflineTimeout := lastConnectivity.Before(now.Add(time.Duration(n.Config().OfflineBeforeStartingHotspotMinutes)*-1)) &&
pModeChange.Before(now.Add(time.Duration(n.Config().OfflineBeforeStartingHotspotMinutes)*-1))
// not in provisioning mode, so start it if not configured (/etc/viam.json)
// OR as long as we've been offline AND out of provisioning mode for at least OfflineTimeout (2 minute default)
if !isConfigured || hitOfflineTimeout {
Expand All @@ -775,7 +780,7 @@ func (n *Networking) mainLoop(ctx context.Context) {
}

func (n *Networking) doReboot(ctx context.Context) bool {
n.logger.Infof("device has been offline for more than %s, rebooting", time.Duration(n.cfg.DeviceRebootAfterOfflineMinutes))
n.logger.Infof("device has been offline for more than %s, rebooting", time.Duration(n.Config().DeviceRebootAfterOfflineMinutes))
cmd := exec.Command("systemctl", "reboot")
output, err := cmd.CombinedOutput()
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion uninstall.sh
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ systemctl daemon-reload
rm -v /usr/local/bin/viam-server

# configs
rm -v /etc/viam.json /etc/viam-provisioning.json
rm -v /etc/viam.json /etc/viam-provisioning.json /etc/viam-defaults.json

# root/viamdir
rm -vr /root/.viam/
Expand Down
6 changes: 4 additions & 2 deletions utils/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,9 @@ func LoadConfigFromCache() (AgentConfig, error) {
//nolint:gosec
cacheBytes, err := os.ReadFile(cachePath)
if err != nil {
if !errors.Is(err, fs.ErrNotExist) {
if errors.Is(err, fs.ErrNotExist) {
return StackConfigs(&pb.DeviceAgentConfigResponse{})
} else {
Comment on lines +198 to +200
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 is the actual fix.

cfg, newErr := StackConfigs(&pb.DeviceAgentConfigResponse{})
return cfg, errors.Join(errw.Wrap(err, "reading config cache"), newErr)
}
Expand Down Expand Up @@ -513,6 +515,6 @@ func (t *Timeout) UnmarshalJSON(b []byte) error {
*t = Timeout(tmp)
return nil
default:
return errw.Errorf("invalid duration: %+v", v)
return errw.Errorf("invalid duration: %#v", v)
}
}