Skip to content

Commit

Permalink
App-shared port without DNS server is not necessarily an issue
Browse files Browse the repository at this point in the history
Not having DNS servers configured (while link is OK and IP address
is assigned), is not necessarily a failure for an app-shared interface.
Some applications might not require DNS for external hostnames
and may only use internal DNS servers to resolve the names of other apps
running on the same device. However, we should still issue a warning about
this potential issue.

Signed-off-by: Milan Lenco <milan@zededa.com>
  • Loading branch information
milan-zededa authored and OhmSpectator committed Oct 2, 2024
1 parent 7dd83ea commit e7bb230
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 4 deletions.
4 changes: 4 additions & 0 deletions pkg/pillar/cmd/zedagent/handlemetrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,10 @@ func encodeTestResults(tr types.TestResults) *info.ErrorInfo {
errInfo.Severity = info.Severity_SEVERITY_ERROR
} else {
timestamp = tr.LastSucceeded
if tr.HasWarning() {
errInfo.Description = tr.LastWarning
errInfo.Severity = info.Severity_SEVERITY_WARNING
}
}
if !timestamp.IsZero() {
protoTime, err := ptypes.TimestampProto(timestamp)
Expand Down
30 changes: 30 additions & 0 deletions pkg/pillar/types/conntest.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,23 @@ type TestResults struct {
LastFailed time.Time
LastSucceeded time.Time
LastError string // Set when LastFailed is updated
LastWarning string // test succeeded but there is a potential issue
}

// RecordSuccess records a success
// Keeps the LastFailed in place as history
func (trPtr *TestResults) RecordSuccess() {
trPtr.LastSucceeded = time.Now()
trPtr.LastError = ""
trPtr.LastWarning = ""
}

// RecordSuccessWithWarning records a test success but warns user about a potential issue.
// Keeps the LastFailed in place as history
func (trPtr *TestResults) RecordSuccessWithWarning(warnStr string) {
trPtr.LastSucceeded = time.Now()
trPtr.LastError = ""
trPtr.LastWarning = warnStr
}

// RecordFailure records a failure
Expand All @@ -32,6 +42,7 @@ func (trPtr *TestResults) RecordFailure(errStr string) {
}
trPtr.LastFailed = time.Now()
trPtr.LastError = errStr
trPtr.LastWarning = ""
}

// HasError returns true if there is an error
Expand All @@ -40,18 +51,25 @@ func (trPtr *TestResults) HasError() bool {
return trPtr.LastFailed.After(trPtr.LastSucceeded)
}

// HasWarning returns true if test succeeded but there is a warning reported.
func (trPtr *TestResults) HasWarning() bool {
return !trPtr.HasError() && trPtr.LastWarning != ""
}

// Update uses the src to add info to the results
// If src has newer information for the 'other' part we update that as well.
func (trPtr *TestResults) Update(src TestResults) {
if src.HasError() {
trPtr.LastFailed = src.LastFailed
trPtr.LastError = src.LastError
trPtr.LastWarning = ""
if src.LastSucceeded.After(trPtr.LastSucceeded) {
trPtr.LastSucceeded = src.LastSucceeded
}
} else {
trPtr.LastSucceeded = src.LastSucceeded
trPtr.LastError = ""
trPtr.LastWarning = src.LastWarning
if src.LastFailed.After(trPtr.LastFailed) {
trPtr.LastFailed = src.LastFailed
}
Expand All @@ -63,6 +81,7 @@ func (trPtr *TestResults) Clear() {
trPtr.LastFailed = time.Time{}
trPtr.LastSucceeded = time.Time{}
trPtr.LastError = ""
trPtr.LastWarning = ""
}

// IntfStatusMap - Used to return per-interface test results (success and failures)
Expand Down Expand Up @@ -100,6 +119,17 @@ func (intfMap *IntfStatusMap) RecordFailure(ifName string, errStr string) {
intfMap.StatusMap[ifName] = tr
}

// RecordSuccessWithWarning records a verification success but warns user about
// a potential issue.
func (intfMap *IntfStatusMap) RecordSuccessWithWarning(ifName, warnStr string) {
tr, ok := intfMap.StatusMap[ifName]
if !ok {
tr = TestResults{}
}
tr.RecordSuccessWithWarning(warnStr)
intfMap.StatusMap[ifName] = tr
}

// SetOrUpdateFromMap - Set all the entries from the given per-interface map
// Entries which are not in the source are not modified
func (intfMap *IntfStatusMap) SetOrUpdateFromMap(
Expand Down
10 changes: 8 additions & 2 deletions pkg/pillar/types/dns.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ func (status DeviceNetworkStatus) LogCreate(logBase *base.LogObject) {
// XXX different logobject for a particular port?
logObject.CloneAndAddField("ifname", p.IfName).
AddField("last-error", p.LastError).
AddField("last-warning", p.LastWarning).
AddField("last-succeeded", p.LastSucceeded).
AddField("last-failed", p.LastFailed).
Noticef("DeviceNetworkStatus port create")
Expand Down Expand Up @@ -163,16 +164,20 @@ func (status DeviceNetworkStatus) LogModify(logBase *base.LogObject, old interfa
if p.HasError() != op.HasError() ||
p.LastFailed != op.LastFailed ||
p.LastSucceeded != op.LastSucceeded ||
p.LastError != op.LastError {
p.LastError != op.LastError ||
p.LastWarning != op.LastWarning {
logData := logObject.CloneAndAddField("ifname", p.IfName).
AddField("last-error", p.LastError).
AddField("last-warning", p.LastWarning).
AddField("last-succeeded", p.LastSucceeded).
AddField("last-failed", p.LastFailed).
AddField("old-last-error", op.LastError).
AddField("old-last-warning", op.LastWarning).
AddField("old-last-succeeded", op.LastSucceeded).
AddField("old-last-failed", op.LastFailed)
if p.HasError() == op.HasError() &&
p.LastError == op.LastError {
p.LastError == op.LastError &&
p.LastWarning == op.LastWarning {
// if we have success or the same error again, reduce log level
logData.Function("DeviceNetworkStatus port modify")
} else {
Expand All @@ -194,6 +199,7 @@ func (status DeviceNetworkStatus) LogDelete(logBase *base.LogObject) {
// XXX different logobject for a particular port?
logObject.CloneAndAddField("ifname", p.IfName).
AddField("last-error", p.LastError).
AddField("last-warning", p.LastWarning).
AddField("last-succeeded", p.LastSucceeded).
AddField("last-failed", p.LastFailed).
Noticef("DeviceNetworkStatus port delete")
Expand Down
16 changes: 14 additions & 2 deletions pkg/pillar/types/dpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,12 +137,14 @@ func (config DevicePortConfig) LogCreate(logBase *base.LogObject) {
AddField("last-failed", config.LastFailed).
AddField("last-succeeded", config.LastSucceeded).
AddField("last-error", config.LastError).
AddField("last-warning", config.LastWarning).
AddField("state", config.State.String()).
Noticef("DevicePortConfig create")
for _, p := range config.Ports {
// XXX different logobject for a particular port?
logObject.CloneAndAddField("ifname", p.IfName).
AddField("last-error", p.LastError).
AddField("last-warning", p.LastWarning).
AddField("last-succeeded", p.LastSucceeded).
AddField("last-failed", p.LastFailed).
Noticef("DevicePortConfig port create")
Expand All @@ -162,21 +164,25 @@ func (config DevicePortConfig) LogModify(logBase *base.LogObject, old interface{
oldConfig.LastFailed != config.LastFailed ||
oldConfig.LastSucceeded != config.LastSucceeded ||
oldConfig.LastError != config.LastError ||
oldConfig.LastWarning != config.LastWarning ||
oldConfig.State != config.State {

logData := logObject.CloneAndAddField("ports-int64", len(config.Ports)).
AddField("last-failed", config.LastFailed).
AddField("last-succeeded", config.LastSucceeded).
AddField("last-error", config.LastError).
AddField("last-warning", config.LastWarning).
AddField("state", config.State.String()).
AddField("old-ports-int64", len(oldConfig.Ports)).
AddField("old-last-failed", oldConfig.LastFailed).
AddField("old-last-succeeded", oldConfig.LastSucceeded).
AddField("old-last-error", oldConfig.LastError).
AddField("old-last-warning", oldConfig.LastWarning).
AddField("old-state", oldConfig.State.String())
if len(oldConfig.Ports) == len(config.Ports) &&
config.LastFailed == oldConfig.LastFailed &&
config.LastError == oldConfig.LastError &&
config.LastWarning == oldConfig.LastWarning &&
oldConfig.State == config.State &&
config.LastSucceeded.After(oldConfig.LastFailed) &&
oldConfig.LastSucceeded.After(oldConfig.LastFailed) {
Expand All @@ -196,16 +202,20 @@ func (config DevicePortConfig) LogModify(logBase *base.LogObject, old interface{
if p.HasError() != op.HasError() ||
p.LastFailed != op.LastFailed ||
p.LastSucceeded != op.LastSucceeded ||
p.LastError != op.LastError {
p.LastError != op.LastError ||
p.LastWarning != op.LastWarning {
logData := logObject.CloneAndAddField("ifname", p.IfName).
AddField("last-error", p.LastError).
AddField("last-warning", p.LastWarning).
AddField("last-succeeded", p.LastSucceeded).
AddField("last-failed", p.LastFailed).
AddField("old-last-error", op.LastError).
AddField("old-last-warning", op.LastWarning).
AddField("old-last-succeeded", op.LastSucceeded).
AddField("old-last-failed", op.LastFailed)
if p.HasError() == op.HasError() &&
p.LastError == op.LastError {
p.LastError == op.LastError &&
p.LastWarning == op.LastWarning {
// if we have success or the same error again, reduce log level
logData.Function("DevicePortConfig port modify")
} else {
Expand All @@ -223,12 +233,14 @@ func (config DevicePortConfig) LogDelete(logBase *base.LogObject) {
AddField("last-failed", config.LastFailed).
AddField("last-succeeded", config.LastSucceeded).
AddField("last-error", config.LastError).
AddField("last-warning", config.LastWarning).
AddField("state", config.State.String()).
Noticef("DevicePortConfig delete")
for _, p := range config.Ports {
// XXX different logobject for a particular port?
logObject.CloneAndAddField("ifname", p.IfName).
AddField("last-error", p.LastError).
AddField("last-warning", p.LastWarning).
AddField("last-succeeded", p.LastSucceeded).
AddField("last-failed", p.LastFailed).
Noticef("DevicePortConfig port delete")
Expand Down
13 changes: 13 additions & 0 deletions pkg/pillar/zedcloud/send.go
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,19 @@ func VerifyAllIntf(ctx *ZedCloudContext, url string, requiredSuccessCount uint,
continue
}
}
var noDNSErr *types.DNSNotAvailError
if errors.As(err, &noDNSErr) {
// The interface link is up and an IP address is assigned, but no DNS
// server is configured. This is not necessarily a failure for an app-shared
// interface. Some applications might not require DNS for external hostnames
// and may only use internal DNS servers to resolve the names of other apps
// running on the same device. However, we should still issue a warning about
// this potential issue.
if !portStatus.IsMgmt {
verifyRV.IntfStatusMap.RecordSuccessWithWarning(intf, err.Error())
continue
}
}
log.Errorf("Zedcloud un-reachable via interface %s: %s",
intf, err)
if sendErr, ok := err.(*SendError); ok && len(sendErr.Attempts) > 0 {
Expand Down

0 comments on commit e7bb230

Please sign in to comment.