Skip to content

Commit

Permalink
Fix incorrect defer usage
Browse files Browse the repository at this point in the history
Problem:
Using defer inside a loop can lead to resource leaks

Solution:
Judge newer file in the separate function

Signed-off-by: iyear <ljyngup@gmail.com>
  • Loading branch information
iyear authored and brandond committed Nov 1, 2022
1 parent 86be784 commit 3aae7b8
Showing 1 changed file with 40 additions and 25 deletions.
65 changes: 40 additions & 25 deletions pkg/cluster/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -328,35 +328,15 @@ func (c *Cluster) ReconcileBootstrapData(ctx context.Context, buf io.ReadSeeker,
}
logrus.Debugf("Reconciling %s at '%s'", pathKey, path)

f, err := os.Open(path)
updated, newer, err := isNewerFile(path, fileData)
if err != nil {
if os.IsNotExist(err) {
logrus.Warn(path + " doesn't exist. continuing...")
updateDisk = true
continue
}
return errors.Wrapf(err, "reconcile failed to open %s", pathKey)
return errors.Wrapf(err, "failed to get update status of %s", pathKey)
}
defer f.Close()

fData, err := io.ReadAll(f)
if err != nil {
return errors.Wrapf(err, "reconcile failed to read %s", pathKey)
if newer {
newerOnDisk = append(newerOnDisk, path)
}

if !bytes.Equal(fileData.Content, fData) {
updateDisk = true
info, err := f.Stat()
if err != nil {
return errors.Wrapf(err, "reconcile failed to stat %s", pathKey)
}

if info.ModTime().Unix()-fileData.Timestamp.Unix() >= systemTimeSkew {
newerOnDisk = append(newerOnDisk, path)
} else {
logrus.Warn(path + " will be updated from the datastore.")
}
}
updateDisk = updateDisk || updated
}

if c.config.ClusterReset {
Expand Down Expand Up @@ -384,6 +364,41 @@ func (c *Cluster) ReconcileBootstrapData(ctx context.Context, buf io.ReadSeeker,
return nil
}

// isNewerFile compares the file from disk and datastore, and returns
// update status.
func isNewerFile(path string, file bootstrap.File) (updated bool, newerOnDisk bool, _ error) {
f, err := os.Open(path)
if err != nil {
if os.IsNotExist(err) {
logrus.Warn(path + " doesn't exist. continuing...")
return true, false, nil
}
return false, false, errors.Wrapf(err, "reconcile failed to open")
}
defer f.Close()

data, err := io.ReadAll(f)
if err != nil {
return false, false, errors.Wrapf(err, "reconcile failed to read")
}

if bytes.Equal(file.Content, data) {
return false, false, nil
}

info, err := f.Stat()
if err != nil {
return false, false, errors.Wrapf(err, "reconcile failed to stat")
}

if info.ModTime().Unix()-file.Timestamp.Unix() >= systemTimeSkew {
return true, true, nil
}

logrus.Warn(path + " will be updated from the datastore.")
return true, false, nil
}

// httpBootstrap retrieves bootstrap data (certs and keys, etc) from the remote server via HTTP
// and loads it into the ControlRuntimeBootstrap struct. Unlike the storage bootstrap path,
// this data does not need to be decrypted since it is generated on-demand by an existing server.
Expand Down

0 comments on commit 3aae7b8

Please sign in to comment.