Skip to content

Commit

Permalink
Remove DownloaderStatus Pending flags
Browse files Browse the repository at this point in the history
I do not see any practical value of these Pending flags, other than
blocking error update propagation from DownloaderStatus to BlobStatus.
But this can be confusing for the user.

Consider this example:
1. User enters wrong datastore config
2. User adds app config with image from this misconfigured datastore
3. Downloader fails, error is published for the blob/volume/app
4. User fixes datastore config
5. App image download starts and progresses successfully
6. The obsolete error continues being reported until download
   completes, which for large VM images may take quite some time
7. User could get confused and think that he didn't actually fix the
   datastore config, then try further changes, etc.

Signed-off-by: Milan Lenco <[email protected]>
  • Loading branch information
milan-zededa authored and eriknordmark committed Nov 28, 2023
1 parent 86272d7 commit ed98e7d
Show file tree
Hide file tree
Showing 3 changed files with 0 additions and 34 deletions.
12 changes: 0 additions & 12 deletions pkg/pillar/cmd/downloader/downloader.go
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,6 @@ func handleCreate(ctx *downloaderContext, config types.DownloaderConfig,
RefCount: config.RefCount,
Size: config.Size,
LastUse: time.Now(),
PendingAdd: true,
}
status = &status0
} else {
Expand All @@ -401,11 +400,6 @@ func handleModify(ctx *downloaderContext, key string,
config types.DownloaderConfig, status *types.DownloaderStatus,
receiveChan chan<- CancelChannel) {

log.Functionf("handleModify(%s) for %s", status.ImageSha256, status.Name)

status.PendingModify = true
publishDownloaderStatus(ctx, status)

log.Functionf("handleModify(%s) RefCount %d to %d, Expired %v for %s",
status.ImageSha256, status.RefCount, config.RefCount,
status.Expired, status.Name)
Expand All @@ -422,7 +416,6 @@ func handleModify(ctx *downloaderContext, key string,
}
status.LastUse = time.Now()
status.Expired = (status.RefCount == 0) // Start delete handshake
status.ClearPendingStatus()
publishDownloaderStatus(ctx, status)
log.Functionf("handleModify done for %s", config.Name)
}
Expand Down Expand Up @@ -541,7 +534,6 @@ func doDownload(ctx *downloaderContext, config types.DownloaderConfig, status *t
status.ModTime = time.Now()
status.State = types.DOWNLOADED
status.Progress = 100 // Just in case
status.ClearPendingStatus()

// All good
break
Expand All @@ -556,12 +548,8 @@ func handleDelete(ctx *downloaderContext, key string,
status.ImageSha256, status.Name,
status.RefCount, status.LastUse, status.Expired)

status.PendingDelete = true
publishDownloaderStatus(ctx, status)

doDelete(ctx, key, status.Target, status)

status.PendingDelete = false
status.State = types.INITIAL
publishDownloaderStatus(ctx, status)

Expand Down
4 changes: 0 additions & 4 deletions pkg/pillar/cmd/volumemgr/blob.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,6 @@ func downloadBlob(ctx *volumemgrContext, blob *types.BlobStatus) bool {
}
changed = true
}
if ds.Pending() {
log.Tracef("lookupDownloaderStatus Pending for blob %s", blob.Sha256)
return changed
}
if ds.HasError() {
log.Errorf("Received error from downloader for blob %s: %s",
blob.Sha256, ds.Error)
Expand Down
18 changes: 0 additions & 18 deletions pkg/pillar/types/downloadertypes.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,6 @@ type DownloaderStatus struct {
DatastoreIDList []uuid.UUID
Target string // file path where we download the file
Name string
PendingAdd bool
PendingModify bool
PendingDelete bool
RefCount uint // Zero means not downloaded
LastUse time.Time // When RefCount dropped to zero
Expired bool // Handshake to client
Expand All @@ -128,20 +125,6 @@ func (status DownloaderStatus) Key() string {
return status.ImageSha256
}

func (status DownloaderStatus) Pending() bool {
return status.PendingAdd || status.PendingModify || status.PendingDelete
}

// ClearPendingStatus : Clear Pending Status for DownloaderStatus
func (status *DownloaderStatus) ClearPendingStatus() {
if status.PendingAdd {
status.PendingAdd = false
}
if status.PendingModify {
status.PendingModify = false
}
}

// HandleDownloadFail : Do Failure specific tasks
func (status *DownloaderStatus) HandleDownloadFail(errStr string, retryTime time.Duration, cancelled bool) {
errDescription := ErrorDescription{
Expand All @@ -153,7 +136,6 @@ func (status *DownloaderStatus) HandleDownloadFail(errStr string, retryTime time
errDescription.ErrorRetryCondition = fmt.Sprintf("Will retry in %s; have retried %d times", retryTime, status.RetryCount)
}
status.SetErrorDescription(errDescription)
status.ClearPendingStatus()
}

// LogCreate :
Expand Down

0 comments on commit ed98e7d

Please sign in to comment.