Skip to content

Commit

Permalink
move prune commands to single container
Browse files Browse the repository at this point in the history 
Issue : with older apporach pruner was crating one container for
        one namespace which says 100 container for 100 namespaces
Fix : We need to think of a proper appraoch to tackel this but for
      the time being moving all the prune commands to one container

Signed-off-by: Pradeep Kumar <pradkuma@redhat.com>
  • Loading branch information
@pradeepitm12 @tekton-robot
pradeepitm12 authored and tekton-robot committed May 31, 2022
1 parent 2cd4476 commit 2133763
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 50 deletions.
67 changes: 39 additions & 28 deletions pkg/reconciler/common/prune.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,15 +128,15 @@ func Prune(ctx context.Context, k kubernetes.Interface, tC *v1alpha1.TektonConfi
return err
}
if pruningNamespaces.commonScheduleNs != nil && len(pruningNamespaces.commonScheduleNs) > 0 {
jobs := pruner.createAllJobContainers(pruningNamespaces.commonScheduleNs)
jobs := getJobContainer(generateAllPruneConfig(pruningNamespaces.commonScheduleNs), pruner.targetNamespace, pruner.tknImage)
if err := pruner.checkAndCreate(ctx, "", tC.Spec.Pruner.Schedule, jobs, pruneCronLabel, pruningNamespaces.configChanged); err != nil {
logger.Error("failed to create cronjob ", err)
}
}

if pruningNamespaces.uniqueScheduleNS != nil {
for ns, con := range pruningNamespaces.uniqueScheduleNS {
jobs := pruner.createJobContainers(con, ns)
jobs := getJobContainer(generatePruneConfigPerNamespace(con, ns), ns, pruner.tknImage)
listOpt := pruneCronNsLabel + "=" + ns
if err := pruner.checkAndCreate(ctx, ns, con.config.Schedule, jobs, listOpt, pruningNamespaces.configChanged); err != nil {
logger.Error("failed to create cronjob ", err)
Expand Down Expand Up @@ -240,30 +240,26 @@ func prunableNamespaces(ctx context.Context, k kubernetes.Interface, defaultPrun
return prunableNs, nil
}

func (pruner *Pruner) createAllJobContainers(nsConfig map[string]*pruneConfigPerNS) []corev1.Container {
var containers []corev1.Container
func generateAllPruneConfig(nsConfig map[string]*pruneConfigPerNS) string {
var cmds string
for ns, con := range nsConfig {
jobContainers := pruner.createJobContainers(con, ns)
containers = append(containers, jobContainers...)
cmd := generatePruneConfigPerNamespace(con, ns)
cmds = cmds + " " + cmd
}
return containers
return cmds
}

func (pruner *Pruner) createJobContainers(nsConfig *pruneConfigPerNS, ns string) []corev1.Container {
var containers []corev1.Container

cmdArgs := pruneCommand(nsConfig, ns)
func getJobContainer(cmdArgs, ns string, tknImage string) []corev1.Container {
cmd := "function prune() { n=$1; a=$2; resources=$3; old_ifs=\" \"; IFS=\",\"; for r in $resources; do tkn $r delete -n=$n $a -f; done; IFS=$old_ifs; }; echo $conf; for c in $*; do ns=$(echo $c | cut -d \";\" -f 1); args=$(echo $c | cut -d \";\" -f 2); resources=$(echo $c | cut -d \";\" -f 3); prune $ns $args $resources; done;"
containerName := SimpleNameGenerator.RestrictLengthWithRandomSuffix("pruner-tkn-" + ns)
container := corev1.Container{
Name: containerName,
Image: pruner.tknImage,
Command: []string{"/bin/sh", "-c"},
Args: []string{cmdArgs},
Image: tknImage,
Command: []string{"/bin/sh", "-c", cmd},
Args: []string{"-s", cmdArgs},
TerminationMessagePolicy: "FallbackToLogsOnError",
}
containers = append(containers, container)

return containers
return []corev1.Container{container}
}

func (pruner *Pruner) checkAndCreate(ctx context.Context, uniquePruneNs, schedule string, pruneContainers []corev1.Container, listOpt string, configChanged bool) error {
Expand Down Expand Up @@ -344,20 +340,35 @@ func createCronJob(ctx context.Context, kc kubernetes.Interface, cronName, targe
return nil
}

func pruneCommand(pru *pruneConfigPerNS, ns string) string {
var cmdArgs string
for _, resource := range pru.config.Resources {
// generatePruneConfigPerNamespace takes config `pruneConfigPerNS` per namespace and return strings like
// ns-one;--keep=5;pipelinerun
// ns-two;--keep=3;pipelinerun,taskrun
// ns-three;--keep=2;taskrun
// these configs are passed as space seperated string argument to pod container spec
// for each of this namespaced config below commands are generated
// tkn pipelinerun delete --keep=5 -n=ns-one -f ;
// tkn pipelinerun delete --keep=3 -n=ns-two -f ;
// tkn taskrun delete --keep=3 -n=ns-two -f ;
// tkn taskrun delete --keep=3 -n=ns-three -f ;
func generatePruneConfigPerNamespace(pru *pruneConfigPerNS, ns string) string {
var keepCmd string
if pru.config.Keep != nil {
keepCmd = "--keep=" + fmt.Sprint(*pru.config.Keep)
}
if pru.config.Keep == nil && pru.config.KeepSince != nil {
keepCmd = "--keep-since=" + fmt.Sprint(*pru.config.KeepSince)
}
cmdArgs := ns + ";" + keepCmd + ";"
var resources string
for i, resource := range pru.config.Resources {
res := strings.TrimSpace(resource)
var keepCmd string
if pru.config.Keep != nil {
keepCmd = "--keep=" + fmt.Sprint(*pru.config.Keep)
}
if pru.config.Keep == nil && pru.config.KeepSince != nil {
keepCmd = "--keep-since=" + fmt.Sprint(*pru.config.KeepSince)
if i < len(pru.config.Resources)-1 {
resources = resources + res + ","
} else {
resources = resources + res
}
cmd := "tkn " + strings.ToLower(res) + " delete " + keepCmd + " -n=" + ns + " -f ; "
cmdArgs = cmdArgs + cmd
}
cmdArgs = cmdArgs + resources
return cmdArgs
}

Expand Down
46 changes: 24 additions & 22 deletions pkg/reconciler/common/prune_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package common
import (
"context"
"errors"
"fmt"
"os"
"strings"
"testing"
Expand Down Expand Up @@ -141,8 +140,8 @@ func TestPruneCommands(t *testing.T) {
keep := uint(2)
keepsince := uint(300)
expected := []string{
"tkn pipelinerun delete --keep=2 -n=ns -f ; tkn taskrun delete --keep=2 -n=ns -f ; ",
"tkn pipelinerun delete --keep-since=300 -n=ns -f ; tkn taskrun delete --keep-since=300 -n=ns -f ; ",
"ns;--keep=2;pipelinerun,taskrun",
"ns;--keep-since=300;pipelinerun,taskrun",
}
ns := "ns"
configs := []*pruneConfigPerNS{
Expand All @@ -165,7 +164,7 @@ func TestPruneCommands(t *testing.T) {
}

for i, config := range configs {
cmd := pruneCommand(config, ns)
cmd := generatePruneConfigPerNamespace(config, ns)
assert.Equal(t, cmd, expected[i])
}
}
Expand Down Expand Up @@ -210,40 +209,43 @@ func TestAnnotationCmd(t *testing.T) {
&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "ns-ten", Annotations: annoResourceTrPr}},
&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "ns-thirteen", Annotations: annoStrategyKeep}},
)
expected := map[string]string{
"one-" + scheduleCommon: "tkn pipelinerun delete --keep=3 -n=ns-one -f ; ",
"two-" + scheduleUnique: "tkn pipelinerun delete --keep=3 -n=ns-two -f ; ",
"four-" + scheduleCommon: "tkn pipelinerun delete --keep=3 -n=ns-four -f ; ",
"seven-" + scheduleCommon: "tkn pipelinerun delete --keep-since=3200 -n=ns-seven -f ; ",
"eight-" + scheduleCommon: "tkn pipelinerun delete --keep=5 -n=ns-eight -f ; ",
"nine-" + scheduleCommon: "tkn taskrun delete --keep=3 -n=ns-nine -f ; ",
"ten-" + scheduleCommon: "tkn taskrun delete --keep=3 -n=ns-ten -f ; tkn pipelinerun delete --keep=3 -n=ns-ten -f ; ",
"ns-thirteen" + scheduleCommon: "tkn pipelinerun delete --keep=50 -n=ns-thirteen -f ; ",
}
os.Setenv(JobsTKNImageName, "some")

err := Prune(context.TODO(), client, config)
if err != nil {
assert.Error(t, err, "unable to get ns list")
assert.Error(t, err, "pruning failed ")
}
cronjobs, err := client.BatchV1().CronJobs("").List(context.TODO(), metav1.ListOptions{})
if err != nil {
assert.Error(t, err, "unable to get ns list")
}
// Only one ns with unique schedule than default
if len(cronjobs.Items) != 2 {
assert.Error(t, err, "unable to get ns list")
assert.Error(t, err, "number of cronjobs created is not right")
}
expected := map[string]struct{}{
"ns-two;--keep=3;pipelinerun": {},
"ns-eight;--keep=5;pipelinerun": {},
"ns-four;--keep=3;pipelinerun": {},
"ns-nine;--keep=3;taskrun": {},
"ns-one;--keep=3;pipelinerun": {},
"ns-seven;--keep-since=3200;pipelinerun": {},
"ns-ten;--keep=3;taskrun,pipelinerun": {},
"ns-thirteen;--keep=50;pipelinerun": {},
}
for _, cronjob := range cronjobs.Items {
assert.Equal(t, len(cronjob.Spec.JobTemplate.Spec.Template.Spec.Containers), 1)
for _, container := range cronjob.Spec.JobTemplate.Spec.Template.Spec.Containers {
if _, ok := expected[container.Name[14:len(container.Name)-5]+cronjob.Spec.Schedule]; ok {
if expected[container.Name[14:len(container.Name)-5]+cronjob.Spec.Schedule] != strings.Join(container.Args, " ") {
msg := fmt.Sprintf("expected : %s\n actual : %s \n", expected[container.Name[14:len(container.Name)-5]+cronjob.Spec.Schedule], strings.Join(container.Args, " "))
assert.Error(t, errors.New("command created is not as expected"), msg)
args := strings.Split(container.Args[1], " ")
if len(args) == 1 {
if _, ok := expected[args[0]]; !ok {
assert.Error(t, err, "expected args not found")
}
}
if container.Name == "ns-six" {
assert.Error(t, errors.New("Should not be created as Ns have skip prune annotation"), "")
for _, arg := range args[1:] {
if _, ok := expected[arg]; !ok {
assert.Error(t, errors.New("not found"), "expected command not found")
}
}
}
}
Expand Down

0 comments on commit 2133763

Please sign in to comment.