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.

Sign up for GitHub

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

Remove stack when directory is renamed #5250

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Remove stack when directory is renamed #5250

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ca6b696
Store manifests dir directly in applier manager
twz123 Jun 20, 2024
aa704c5
Remove stack when directory is renamed
twz123 Jun 25, 2024
80aff81
Add test for folder renames in applier manager
twz123 Jun 25, 2024
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: 1 addition & 1 deletion cmd/controller/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ func (c *command) start(ctx context.Context) error {

if !slices.Contains(c.DisableComponents, constant.ApplierManagerComponentName) {
nodeComponents.Add(ctx, &applier.Manager{
K0sVars: c.K0sVars,
ManifestsDir: c.K0sVars.ManifestsDir,
KubeClientFactory: adminClientFactory,
IgnoredStacks: []string{
controller.ClusterConfigStackName,
Expand Down
11 changes: 5 additions & 6 deletions pkg/applier/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import (
"github.com/k0sproject/k0s/internal/pkg/file"
"github.com/k0sproject/k0s/pkg/component/controller/leaderelector"
"github.com/k0sproject/k0s/pkg/component/manager"
"github.com/k0sproject/k0s/pkg/config"
"github.com/k0sproject/k0s/pkg/constant"
kubeutil "github.com/k0sproject/k0s/pkg/kubernetes"

Expand All @@ -41,7 +40,7 @@ import (

// Manager is the Component interface wrapper for Applier
type Manager struct {
K0sVars *config.CfgVars
ManifestsDir string
IgnoredStacks []string
KubeClientFactory kubeutil.ClientFactoryInterface

Expand All @@ -62,12 +61,12 @@ type stack = struct {

// Init initializes the Manager
func (m *Manager) Init(ctx context.Context) error {
err := dir.Init(m.K0sVars.ManifestsDir, constant.ManifestsDirMode)
err := dir.Init(m.ManifestsDir, constant.ManifestsDirMode)
if err != nil {
return fmt.Errorf("failed to create manifest bundle dir %s: %w", m.K0sVars.ManifestsDir, err)
return fmt.Errorf("failed to create manifest bundle dir %s: %w", m.ManifestsDir, err)
}
m.log = logrus.WithField("component", constant.ApplierManagerComponentName)
m.bundleDir = m.K0sVars.ManifestsDir
m.bundleDir = m.ManifestsDir

m.LeaderElector.AddAcquiredLeaseCallback(func() {
ctx, cancel := context.WithCancelCause(ctx)
Expand Down Expand Up @@ -151,7 +150,7 @@ func (m *Manager) runWatchers(ctx context.Context) {
if dir.IsDirectory(event.Name) {
m.createStack(ctx, stacks, event.Name)
Copy link
Member Author

@twz123 twz123 Nov 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

In the Windows unit test logs, I see paths in the logs like this: stack="C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\TestManager_Rename2408267215\\001/lorem". Note the last slash, which should IMO be a backslash. IIUC the logged value is coming from fsnotify, so something is not normalizing path names there. Note that the slash is a valid separator on Windows, too, albeit not the canonical one.

}
case fsnotify.Remove:
Copy link
Member Author

Choose a reason for hiding this comment

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

Interesting. The unit test fails on Windows, whereas it works well on Linux. I suspect this is due to the implicit event order which is probably different between Windows and Linux. I can imagine how the Create event for the new name happening before the Rename event for the old name will screw things up. Maybe the fix is more involved than simply reacting on Rename events, as well.

case fsnotify.Remove, fsnotify.Rename:
m.removeStack(ctx, stacks, event.Name)
}

Expand Down
80 changes: 65 additions & 15 deletions pkg/applier/manager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ import (
"testing"
"time"

"github.com/k0sproject/k0s/internal/pkg/file"
"github.com/k0sproject/k0s/internal/testutil"
"github.com/k0sproject/k0s/pkg/applier"
"github.com/k0sproject/k0s/pkg/component/controller/leaderelector"
"github.com/k0sproject/k0s/pkg/config"
"github.com/k0sproject/k0s/pkg/constant"
"github.com/k0sproject/k0s/pkg/kubernetes/watch"

Expand All @@ -49,19 +49,18 @@ import (
)

func TestManager_AppliesStacks(t *testing.T) {
k0sVars, err := config.NewCfgVars(nil, t.TempDir())
require.NoError(t, err)
manifestsDir := t.TempDir()
leaderElector := leaderelector.Dummy{Leader: true}
clients := testutil.NewFakeClientFactory()

underTest := applier.Manager{
K0sVars: k0sVars,
ManifestsDir: manifestsDir,
KubeClientFactory: clients,
LeaderElector: &leaderElector,
}

// A stack that exists on disk before the manager is started.
before := filepath.Join(k0sVars.ManifestsDir, "before")
before := filepath.Join(manifestsDir, "before")
require.NoError(t, os.MkdirAll(before, constant.ManifestsDirMode))
require.NoError(t, os.WriteFile(filepath.Join(before, "before.yaml"), []byte(`
apiVersion: v1
Expand Down Expand Up @@ -89,7 +88,7 @@ data: {}
)

// A stack that is created on disk after the manager has started.
after := filepath.Join(k0sVars.ManifestsDir, "after")
after := filepath.Join(manifestsDir, "after")
require.NoError(t, os.MkdirAll(after, constant.ManifestsDirMode))
require.NoError(t, os.WriteFile(filepath.Join(after, "after.yaml"), []byte(`
apiVersion: v1
Expand All @@ -111,19 +110,18 @@ data: {}
}

func TestManager_IgnoresStacks(t *testing.T) {
k0sVars, err := config.NewCfgVars(nil, t.TempDir())
require.NoError(t, err)
manifestsDir := t.TempDir()
leaderElector := leaderelector.Dummy{Leader: true}
clients := testutil.NewFakeClientFactory()

underTest := applier.Manager{
K0sVars: k0sVars,
ManifestsDir: manifestsDir,
IgnoredStacks: []string{"ignored"},
KubeClientFactory: clients,
LeaderElector: &leaderElector,
}

ignored := filepath.Join(k0sVars.ManifestsDir, "ignored")
ignored := filepath.Join(manifestsDir, "ignored")
require.NoError(t, os.MkdirAll(ignored, constant.ManifestsDirMode))
require.NoError(t, os.WriteFile(filepath.Join(ignored, "ignored.yaml"), []byte(`
apiVersion: v1
Expand All @@ -144,6 +142,7 @@ data: {}

var content []byte
require.EventuallyWithT(t, func(t *assert.CollectT) {
var err error
content, err = os.ReadFile(filepath.Join(ignored, "ignored.txt"))
assert.NoError(t, err)
}, 10*time.Second, 100*time.Millisecond)
Expand All @@ -161,6 +160,61 @@ data: {}
}
}

func TestManager_Rename(t *testing.T) {
cf := testutil.NewFakeClientFactory()
manifestsDir := t.TempDir()
leaderElector := leaderelector.Dummy{}

underTest := applier.Manager{
ManifestsDir: manifestsDir,
KubeClientFactory: cf,
LeaderElector: &leaderElector,
}

require.NoError(t, underTest.Init(context.TODO()))
require.NoError(t, underTest.Start(context.TODO()))
defer func() { assert.NoError(t, underTest.Stop()) }()
leaderElector.Leader = true
require.NoError(t, leaderElector.Start(context.TODO()))

theMap, err := yaml.Marshal(&map[string]any{
"apiVersion": "v1", "kind": "ConfigMap",
"metadata": map[string]any{"name": "the-map"},
})
require.NoError(t, err)
require.NoError(t, os.Mkdir(filepath.Join(manifestsDir, "lorem"), 0755))
require.NoError(t, file.AtomicWithTarget(filepath.Join(manifestsDir, "lorem", "the-map.yaml")).Write(theMap))
t.Log("Waiting for stack to be applied")
require.EventuallyWithT(t, func(t *assert.CollectT) {
configMaps, err := cf.Client.CoreV1().ConfigMaps(metav1.NamespaceAll).List(context.TODO(), metav1.ListOptions{})
if assert.NoError(t, err) && assert.Len(t, configMaps.Items, 1) {
cm := &configMaps.Items[0]
assert.Equal(t, "the-map", cm.Name)
assert.Subset(t, cm.Labels, map[string]string{"k0s.k0sproject.io/stack": "lorem"})
}
}, 20*time.Second, 350*time.Millisecond)

require.NoError(t, os.Rename(filepath.Join(manifestsDir, "lorem"), filepath.Join(manifestsDir, "ipsum")))
t.Log("Waiting for stack to be changed")
require.EventuallyWithT(t, func(t *assert.CollectT) {
configMaps, err := cf.Client.CoreV1().ConfigMaps(metav1.NamespaceAll).List(context.TODO(), metav1.ListOptions{})
if assert.NoError(t, err) && assert.Len(t, configMaps.Items, 1) {
cm := &configMaps.Items[0]
assert.Equal(t, "the-map", cm.Name)
assert.Subset(t, cm.Labels, map[string]string{"k0s.k0sproject.io/stack": "ipsum"})
}
}, 20*time.Second, 350*time.Millisecond)

require.NoError(t, os.Rename(filepath.Join(manifestsDir, "ipsum"), filepath.Join(t.TempDir(), "moved-away")))
t.Log("Waiting for stack to be deleted")
require.EventuallyWithT(t, func(t *assert.CollectT) {
configMaps, err := cf.Client.CoreV1().ConfigMaps(metav1.NamespaceAll).List(context.TODO(), metav1.ListOptions{})
if assert.NoError(t, err) {
assert.Empty(t, configMaps.Items)
}
}, 20*time.Second, 350*time.Millisecond)
}

//go:embed testdata/manager_test/*
var managerTestData embed.FS

Expand All @@ -169,16 +223,12 @@ func TestManager(t *testing.T) {

dir := t.TempDir()

cfg := &config.CfgVars{
ManifestsDir: dir,
}

fakes := kubeutil.NewFakeClientFactory()

le := new(mockLeaderElector)

manager := &applier.Manager{
K0sVars: cfg,
ManifestsDir: dir,
KubeClientFactory: fakes,
LeaderElector: le,
}
Expand Down
Loading