Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 0 additions & 3 deletions pkg/operator/staticpod/controller/prune/prune_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ const (
func NewPruneController(
targetNamespace string,
podResourcePrefix string,
certDir string,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is no longer needed, but it also breaks the API 🤷🏻‍♂️

command []string,
configMapGetter corev1client.ConfigMapsGetter,
podGetter corev1client.PodsGetter,
Expand All @@ -62,7 +61,6 @@ func NewPruneController(
c := &PruneController{
targetNamespace: targetNamespace,
podResourcePrefix: podResourcePrefix,
certDir: certDir,
command: command,

operatorClient: operatorClient,
Expand Down Expand Up @@ -212,7 +210,6 @@ func (c *PruneController) ensurePrunePod(ctx context.Context, recorder events.Re
fmt.Sprintf("--max-eligible-revision=%d", maxEligibleRevision),
fmt.Sprintf("--protected-revisions=%s", revisionsToString(protectedRevisions)),
fmt.Sprintf("--resource-dir=%s", "/etc/kubernetes/static-pod-resources"),
fmt.Sprintf("--cert-dir=%s", c.certDir),
fmt.Sprintf("--static-pod-name=%s", c.podResourcePrefix),
)

Expand Down
14 changes: 7 additions & 7 deletions pkg/operator/staticpod/controller/prune/prune_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package prune
import (
"context"
"fmt"
clocktesting "k8s.io/utils/clock/testing"
"strconv"
"strings"
"testing"
Expand All @@ -15,6 +14,7 @@ import (
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/client-go/kubernetes/fake"
ktesting "k8s.io/client-go/testing"
clocktesting "k8s.io/utils/clock/testing"

operatorv1 "github.com/openshift/api/operator/v1"
"github.com/openshift/library-go/pkg/controller/factory"
Expand Down Expand Up @@ -58,7 +58,7 @@ func TestSync(t *testing.T) {
objects: []int32{1, 2, 3, 4},
expectedObjects: []int32{2, 4},
expectedPrunePod: true,
expectedPruneArgs: "-v=4 --max-eligible-revision=4 --protected-revisions=2,4 --resource-dir=/etc/kubernetes/static-pod-resources --cert-dir= --static-pod-name=test-pod",
expectedPruneArgs: "-v=4 --max-eligible-revision=4 --protected-revisions=2,4 --resource-dir=/etc/kubernetes/static-pod-resources --static-pod-name=test-pod",
},
{
name: "prunes api resources with multiple nodes based on failedLimit 1, succeedLimit 1",
Expand Down Expand Up @@ -87,7 +87,7 @@ func TestSync(t *testing.T) {
objects: []int32{1, 2, 3, 4, 5, 6},
expectedObjects: []int32{2, 3, 4, 5, 6},
expectedPrunePod: true,
expectedPruneArgs: "-v=4 --max-eligible-revision=5 --protected-revisions=2,3,4,5 --resource-dir=/etc/kubernetes/static-pod-resources --cert-dir= --static-pod-name=test-pod",
expectedPruneArgs: "-v=4 --max-eligible-revision=5 --protected-revisions=2,3,4,5 --resource-dir=/etc/kubernetes/static-pod-resources --static-pod-name=test-pod",
},
{
name: "prunes api resources without nodes",
Expand Down Expand Up @@ -124,7 +124,7 @@ func TestSync(t *testing.T) {
objects: []int32{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12},
expectedObjects: []int32{2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12},
expectedPrunePod: true,
expectedPruneArgs: "-v=4 --max-eligible-revision=10 --protected-revisions=2,3,4,5,6,7,8,9,10 --resource-dir=/etc/kubernetes/static-pod-resources --cert-dir= --static-pod-name=test-pod",
expectedPruneArgs: "-v=4 --max-eligible-revision=10 --protected-revisions=2,3,4,5,6,7,8,9,10 --resource-dir=/etc/kubernetes/static-pod-resources --static-pod-name=test-pod",
},
{
name: "prunes api resources based on failedLimit 2, succeedLimit 3 and all relevant revisions set",
Expand All @@ -147,7 +147,7 @@ func TestSync(t *testing.T) {
objects: int32Range(1, 50),
expectedObjects: []int32{8, 9, 10, 19, 20, 28, 29, 30, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50},
expectedPrunePod: true,
expectedPruneArgs: "-v=4 --max-eligible-revision=40 --protected-revisions=8,9,10,19,20,28,29,30,38,39,40 --resource-dir=/etc/kubernetes/static-pod-resources --cert-dir= --static-pod-name=test-pod",
expectedPruneArgs: "-v=4 --max-eligible-revision=40 --protected-revisions=8,9,10,19,20,28,29,30,38,39,40 --resource-dir=/etc/kubernetes/static-pod-resources --static-pod-name=test-pod",
},
{
name: "prunes api resources based on failedLimit 0, succeedLimit 0",
Expand All @@ -170,7 +170,7 @@ func TestSync(t *testing.T) {
objects: int32Range(1, 50),
expectedObjects: []int32{6, 7, 8, 9, 10, 16, 17, 18, 19, 20, 26, 27, 28, 29, 30, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50},
expectedPrunePod: true,
expectedPruneArgs: "-v=4 --max-eligible-revision=40 --protected-revisions=6,7,8,9,10,16,17,18,19,20,26,27,28,29,30,36,37,38,39,40 --resource-dir=/etc/kubernetes/static-pod-resources --cert-dir= --static-pod-name=test-pod",
expectedPruneArgs: "-v=4 --max-eligible-revision=40 --protected-revisions=6,7,8,9,10,16,17,18,19,20,26,27,28,29,30,36,37,38,39,40 --resource-dir=/etc/kubernetes/static-pod-resources --static-pod-name=test-pod",
},
{
name: "protects all",
Expand Down Expand Up @@ -302,7 +302,7 @@ func TestSync(t *testing.T) {
objects: []int32{1, 2, 3, 4, 5, 6, 7, 8, 9, 10},
expectedObjects: []int32{5, 6, 7, 8, 9, 10},
expectedPrunePod: true,
expectedPruneArgs: "-v=4 --max-eligible-revision=5 --protected-revisions=5 --resource-dir=/etc/kubernetes/static-pod-resources --cert-dir= --static-pod-name=test-pod",
expectedPruneArgs: "-v=4 --max-eligible-revision=5 --protected-revisions=5 --resource-dir=/etc/kubernetes/static-pod-resources --static-pod-name=test-pod",
},
}
for _, tc := range tests {
Expand Down
1 change: 0 additions & 1 deletion pkg/operator/staticpod/controllers.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,6 @@ func (b *staticPodOperatorControllerBuilder) ToControllers() (manager.Controller
manager.WithController(prune.NewPruneController(
b.operandNamespace,
b.staticPodPrefix,
b.certDir,
b.pruneCommand,
configMapClient,
podClient,
Expand Down
47 changes: 3 additions & 44 deletions pkg/operator/staticpod/prune/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,9 @@ import (
"fmt"
"os"
"path"
"path/filepath"
"slices"
"strconv"
"strings"
"time"

"github.com/davecgh/go-spew/spew"
"github.com/spf13/cobra"
Expand All @@ -21,7 +19,6 @@ type PruneOptions struct {
ProtectedRevisions []int

ResourceDir string
CertDir string
StaticPodName string
}

Expand Down Expand Up @@ -58,7 +55,8 @@ func (o *PruneOptions) AddFlags(fs *pflag.FlagSet) {
fs.IntSliceVar(&o.ProtectedRevisions, "protected-revisions", o.ProtectedRevisions, "list of revision IDs to preserve (not delete)")
fs.StringVar(&o.ResourceDir, "resource-dir", o.ResourceDir, "directory for all files supporting the static pod manifest")
fs.StringVar(&o.StaticPodName, "static-pod-name", o.StaticPodName, "name of the static pod")
fs.StringVar(&o.CertDir, "cert-dir", o.CertDir, "directory for all certs")
fs.String("cert-dir", "", "directory for all certs")
fs.MarkDeprecated("cert-dir", "certificate directory is now being pruned in relevant sync loops") //nolint:errcheck
}

func (o *PruneOptions) Validate() error {
Expand Down Expand Up @@ -112,44 +110,5 @@ func (o *PruneOptions) Run() error {
return err
}
}

// prune any temporary certificate files
// we do create temporary files to atomically "write" various certificates to disk
// usually, these files are short-lived because they are immediately renamed, the following loop removes old/unused/dangling files
//
// the temporary files have the following form:
// /etc/kubernetes/static-pod-resources/kube-apiserver-certs/configmaps/control-plane-node-kubeconfig/kubeconfig.tmp753375784
// /etc/kubernetes/static-pod-resources/kube-apiserver-certs/secrets/service-network-serving-certkey/tls.key.tmp643092404
if len(o.CertDir) == 0 {
return nil
}

// If the cert dir does not exist, do nothing.
// The dir will get eventually created by an installer pod.
if _, err := os.Stat(path.Join(o.ResourceDir, o.CertDir)); os.IsNotExist(err) {
klog.Infof("Skipping %s as it does not exist", path.Join(o.ResourceDir, o.CertDir))
return nil
}

return filepath.Walk(path.Join(o.ResourceDir, o.CertDir),
func(filePath string, info os.FileInfo, err error) error {
if err != nil {
return err
}
if info.IsDir() {
return nil
}
// info.Name() gives just a filename like tls.key or tls.key.tmp643092404
if !strings.Contains(info.Name(), ".tmp") {
return nil
}
if time.Now().Sub(info.ModTime()) > 30*time.Minute {
klog.Infof("Removing %s, the last time it was modified was %v", filePath, info.ModTime())
if err := os.RemoveAll(filePath); err != nil {
return err
}
}
return nil
},
)
return nil
}