openyurtio / openyurt

OpenYurt - Extending your native Kubernetes to edge(project under CNCF)
https://openyurt.io
Apache License 2.0
1.7k stars 398 forks source link

[BUG] diskStorage calls ReplaceComponentList function which deletes dir (not recreated again) when contents are empty #2133

Open obitoquilt opened 1 month ago

obitoquilt commented 1 month ago

What happened:

diskStorage calls ReplaceComponentList function which deletes dir (not recreated again) when contents are empty.

For example, i installed cilium charts (version is 1.15.6). The cilium-agent component has a empty dir named /etc/kubernetes/cache/cilium-agent/cilium2announcementpolicies.v2alpha1.cilium.io. After diskStorage calls ReplaceComponentList function, the empty dir is renamed to /etc/kubernetes/cache/cilium-agent/tmp_cilium2announcementpolicies.v2alpha1.cilium.io.

Because the contents are empty, diskStorage does nothing and removes the tmp dir. So the original dir would not be recreated.

func (ds *diskStorage) ReplaceComponentList(component string, gvr schema.GroupVersionResource, namespace string, contents map[storage.Key][]byte) error {
    rootKey, err := ds.KeyFunc(storage.KeyBuildInfo{
        Component: component,
        Resources: gvr.Resource,
        Group:     gvr.Group,
        Version:   gvr.Version,
        Namespace: namespace,
    })
    if err != nil {
        return err
    }
    storageKey := rootKey.(storageKey)

    for key := range contents {
        if !strings.HasPrefix(key.Key(), rootKey.Key()) {
            return storage.ErrInvalidContent
        }
    }

    if !ds.lockKey(storageKey) {
        return storage.ErrStorageAccessConflict
    }
    defer ds.unLockKey(storageKey)

    // 1. mv old dir into tmp_dir when rootKey dir already exists
    absPath := filepath.Join(ds.baseDir, storageKey.Key())
    tmpRootKey := getTmpKey(storageKey)
    tmpPath := filepath.Join(ds.baseDir, tmpRootKey.Key())
    if !fs.IfExists(absPath) {
        if err := ds.fsOperator.CreateDir(absPath); err != nil {
            return fmt.Errorf("could not create dir at %s", absPath)
        }
        if len(contents) == 0 {
            // nothing need to create, so just return
            return nil
        }
    }
    if ok, err := fs.IsDir(absPath); err == nil && !ok {
        return fmt.Errorf("%s is not a dir", absPath)
    } else if err != nil {
        return fmt.Errorf("could not check the path %s, %v", absPath, err)
    }
    // absPath exists and is a dir
    if err := ds.fsOperator.Rename(absPath, tmpPath); err != nil {
        return err
    }

    // 2. create new file with contents
    // TODO: if error happens, we may need retry mechanism, or add some mechanism to do consistency check.
    for key, data := range contents {
        path := filepath.Join(ds.baseDir, key.Key())
        if err := ds.fsOperator.CreateDir(filepath.Dir(path)); err != nil && err != fs.ErrExists {
            klog.Errorf("could not create dir at %s, %v", filepath.Dir(path), err)
            continue
        }
        if err := ds.fsOperator.CreateFile(path, data); err != nil {
            klog.Errorf("could not write data to %s, %v", path, err)
            continue
        }
        klog.V(4).Infof("[diskStorage] ReplaceComponentList store data at %s", path)
    }

    //  3. delete old tmp dir
    return ds.fsOperator.DeleteDir(tmpPath)

What you expected to happen:

it should be recreate the empty dir

How to reproduce it (as minimally and precisely as possible):

Anything else we need to know?:

Environment:

others

/kind bug

rambohe-ch commented 3 weeks ago

@obitoquilt Thank you for raising the issue. I agree with you that there is a bug of Yurthub storage, would you like to make a pull request to fix this bug?