Skip to content

Commit

Permalink
nto: avoid timeout when there are too many CSV (#731)
Browse files Browse the repository at this point in the history
Sometimes NTO timesout when listing CSVs.
Adding pagination should solve this problem.

Signed-off-by: Jose Luis Ojosnegros Manchón <jojosneg@redhat.com>
  • Loading branch information
jlojosnegros committed Oct 3, 2023
1 parent 0aacded commit 7ec3a92
Show file tree
Hide file tree
Showing 4 changed files with 888 additions and 22 deletions.
34 changes: 12 additions & 22 deletions cmd/cluster-node-tuning-operator/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
paocontroller "github.com/openshift/cluster-node-tuning-operator/pkg/performanceprofile/controller"
"github.com/openshift/cluster-node-tuning-operator/pkg/performanceprofile/controller/performanceprofile/components/machineconfig"
mcov1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1"
olmoperators "github.com/operator-framework/api/pkg/operators/install"
olmv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
Expand Down Expand Up @@ -51,6 +50,8 @@ const (
webhookCertDir = "/apiserver.local.config/certificates"
webhookCertName = "apiserver.crt"
webhookKeyName = "apiserver.key"

performanceOperatorDeploymentName = "performance-operator"
)

var (
Expand Down Expand Up @@ -210,33 +211,22 @@ func removePerformanceOLMOperator(cfg *rest.Config) error {
return err
}

// Register OLM types to the client
olmoperators.Install(k8sclient.Scheme())

var performanceOperatorCSVs []olmv1alpha1.ClusterServiceVersion
csvs := &olmv1alpha1.ClusterServiceVersionList{}
csvSelector := labels.NewSelector()
req, err := labels.NewRequirement(olmv1alpha1.CopiedLabelKey, selection.DoesNotExist, []string{})
if err != nil {
return err
}
csvSelector.Add(*req)
if err := k8sclient.List(context.TODO(), csvs, client.MatchingLabelsSelector{Selector: csvSelector}); err != nil {
if !errors.IsNotFound(err) {
return err
}

//REVIEW - should this be an input parameter? or something configurable?
paginationLimit := uint64(1000)
options := []client.ListOption{
client.MatchingLabelsSelector{Selector: csvSelector},
}
for i := range csvs.Items {
csv := &csvs.Items[i]
deploymentSpecs := csv.Spec.InstallStrategy.StrategySpec.DeploymentSpecs
if deploymentSpecs != nil {
for _, deployment := range deploymentSpecs {
if deployment.Name == "performance-operator" {
performanceOperatorCSVs = append(performanceOperatorCSVs, *csv)
break
}
}
}

performanceOperatorCSVs, err := paocontroller.ListPerformanceOperatorCSVs(k8sclient, options, paginationLimit, performanceOperatorDeploymentName)
if err != nil {
return err
}

subscriptions := &olmv1alpha1.SubscriptionList{}
Expand All @@ -257,7 +247,7 @@ func removePerformanceOLMOperator(cfg *rest.Config) error {
}
subscriptionExists = false
}
klog.Infof("Removing performance-addon-operator related CSV %s/%s", csv.Name, csv.Namespace)
klog.Infof("Removing performance-addon-operator related CSV %s/%s", csv.Namespace, csv.Name)
if err := k8sclient.Delete(context.TODO(), &csv); err != nil {
return err
}
Expand Down
52 changes: 52 additions & 0 deletions pkg/performanceprofile/controller/utils.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package controller

import (
"context"

"k8s.io/apimachinery/pkg/api/errors"
"sigs.k8s.io/controller-runtime/pkg/client"

olmoperators "github.com/operator-framework/api/pkg/operators/install"
olmv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
)

func ListPerformanceOperatorCSVs(k8sclient client.Client, options []client.ListOption, paginationLimit uint64, performanceOperatorDeploymentName string) ([]olmv1alpha1.ClusterServiceVersion, error) {
// Register OLM types to the client
olmoperators.Install(k8sclient.Scheme())
var performanceOperatorCSVs []olmv1alpha1.ClusterServiceVersion

csvs := &olmv1alpha1.ClusterServiceVersionList{}

opts := []client.ListOption{}
copy(opts, options)
opts = append(options, client.Limit(paginationLimit))

for {
if err := k8sclient.List(context.TODO(), csvs, opts...); err != nil {
if !errors.IsNotFound(err) {
return performanceOperatorCSVs, err
}
}
continueToken := csvs.GetContinue()

for i := range csvs.Items {
csv := &csvs.Items[i]
deploymentSpecs := csv.Spec.InstallStrategy.StrategySpec.DeploymentSpecs

for _, deployment := range deploymentSpecs {
if deployment.Name == performanceOperatorDeploymentName {
performanceOperatorCSVs = append(performanceOperatorCSVs, *csv)
break
}
}

}
if continueToken == "" {
// empty token means there is no more elements
break
}
// add new continuation token and keep looking
opts = append(opts, client.Continue(continueToken))
}
return performanceOperatorCSVs, nil
}
190 changes: 190 additions & 0 deletions test/e2e/basic/csvs.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
package e2e

import (
"context"
"fmt"
"math/rand"
"os"
"strconv"
"time"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/wait"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/config"
"sigs.k8s.io/yaml"

olmv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"

paocontroller "github.com/openshift/cluster-node-tuning-operator/pkg/performanceprofile/controller"
)

const (
namespaceName = "cluster-service-version-test-ns"
performanceOperatorDeploymentName = "performance-operator"
)

var _ = Describe("[basic][clusterserviceversion] ClusterServiceVersion listing", Ordered, func() {
var cli client.Client
var paginationLimit uint64
var namespace corev1.Namespace
var baseOptions []client.ListOption
BeforeAll(func() {
var err error

By("Getting new client")
cli, err = newClient()
Expect(err).To(Succeed())

By("Creating new test namespace ...")
randomize := true
namespace, err = setupNamespace(cli, namespaceName, randomize)
Expect(err).To(Succeed())

By(fmt.Sprintf("New test namespace %s created", namespace.Name))

baseOptions = append(baseOptions, client.InNamespace(namespace.Name))
})
AfterAll(func() {
var err error

baseOptions = nil

By(fmt.Sprintf("Delete test namespace (%s) ", namespace.Name))
err = cli.Delete(context.TODO(), &namespace)
Expect(err).To(Succeed())
})

When("there is no CSVs in the cluster", func() {
It("Should list CSVs without timeout", func() {
ret, err := paocontroller.ListPerformanceOperatorCSVs(cli, baseOptions, paginationLimit, performanceOperatorDeploymentName)
Expect(err).To(Succeed())
Expect(ret).To(HaveLen(0))
})
})

When("there are few CSVs in the cluster", func() {
const csvsFileRelativePath = "../testing_manifests/csv-dummy.yaml"
const numberCSVsToCreate = 1000

BeforeAll(func() {
var err error

By("reading file")
b, err := os.ReadFile(csvsFileRelativePath)
Expect(err).To(Succeed())

By("unmarshaling file")
csvsOrig := olmv1alpha1.ClusterServiceVersion{}
err = yaml.Unmarshal(b, &csvsOrig)
Expect(err).To(Succeed())

By(fmt.Sprintf("Create some(%d) CSVs ... (this gonna take some time)", numberCSVsToCreate))
csvsCreated := []types.NamespacedName{}
for idx := 0; idx < numberCSVsToCreate; idx++ {
csvs := csvsOrig.DeepCopy()
csvs.Name = fmt.Sprintf("csvs-test-%06d", idx)
csvs.Namespace = namespace.Name

err = cli.Create(context.TODO(), csvs)
Expect(err).To(Succeed())
key := types.NamespacedName{
Name: csvs.Name,
Namespace: csvs.Namespace,
}
csvsCreated = append(csvsCreated, key)
}
Expect(len(csvsCreated)).To(Equal(numberCSVsToCreate))

//NOTE - As all the resources have been created into a test namespace
// and that namespace is gonna be deleted in the `Describe`'s `AfterAll` node
// there is no need to explicitly delete created CSVSs
})

When("pagination value is set to ONE", func() {
BeforeAll(func() {
By("Setting pagination to ONE")
paginationLimit = 1
})
It("Should list CSVs without timeout event with low pagination", func() {
By(fmt.Sprintf("Listing elements with pagination %d (this gonna take some time)", paginationLimit))
ret, err := paocontroller.ListPerformanceOperatorCSVs(cli, baseOptions, paginationLimit, performanceOperatorDeploymentName)
Expect(err).To(Succeed())
Expect(ret).To(HaveLen(0))
})
})

When("pagination value is set to a much greater value than the CVS number", func() {
BeforeAll(func() {
times := 100
paginationLimit = uint64(times * numberCSVsToCreate)
By(fmt.Sprintf("Setting paginationLimit to (%d), that is %d times number of csvs created (%d)", paginationLimit, times, numberCSVsToCreate))
})
It("Should list CSVs without timeout even with pagination ", func() {
By(fmt.Sprintf("Listing elements with pagination %d", paginationLimit))
ret, err := paocontroller.ListPerformanceOperatorCSVs(cli, baseOptions, paginationLimit, performanceOperatorDeploymentName)
Expect(err).To(Succeed())
Expect(ret).To(HaveLen(0))
})
})
})
})

func newClient() (client.Client, error) {
cfg, err := config.GetConfig()
if err != nil {
return nil, err
}

c, err := client.New(cfg, client.Options{})
return c, err
}

func setupNamespace(cli client.Client, baseName string, randomize bool) (corev1.Namespace, error) {
validationErrors := validation.IsDNS1123Label(baseName)
if len(validationErrors) > 0 {
return corev1.Namespace{}, fmt.Errorf("Invalid value for namespace name (%s). errors: %v ", baseName, validationErrors)
}
name := baseName
if randomize {
// intentionally avoid GenerateName like the k8s e2e framework does
name = randomizeName(baseName)
}
ns := newNamespace(name)
err := cli.Create(context.TODO(), ns)
if err != nil {
return *ns, err
}

// again we do like the k8s e2e framework does and we try to be robust
var updatedNs corev1.Namespace
err = wait.PollImmediate(1*time.Second, 30*time.Second, func() (bool, error) {
err := cli.Get(context.TODO(), client.ObjectKeyFromObject(ns), &updatedNs)
if err != nil {
return false, err
}
return true, nil
})
return updatedNs, err
}
func randomizeName(baseName string) string {
return fmt.Sprintf("%s-%s", baseName, strconv.Itoa(rand.Intn(10000)))
}

func newNamespace(name string) *corev1.Namespace {
return &corev1.Namespace{
TypeMeta: metav1.TypeMeta{
Kind: "Namespace",
APIVersion: "v1",
},
ObjectMeta: metav1.ObjectMeta{
Name: name,
},
}
}

0 comments on commit 7ec3a92

Please sign in to comment.