From 6eaba5ef1c856935f7195f0dfa234be2c46b5c3f Mon Sep 17 00:00:00 2001 From: Francesco Romani Date: Mon, 23 Oct 2023 10:09:25 +0200 Subject: [PATCH 1/2] wait: add helpers for CRDs Add wait functions for CRD created/deleted. Creating and deleting CRDs is expected to be fast, but not immediate. In CI envs, we can have flakes because CRDs are not deleted fast enough and the attempt to re-create thus fails, failing the test. In all practical environments the operation should be so fast that the extra gets/wait time should be negligible - and in general we do this steps very rarely, in the order of 1-2 times per *cluster* lifespan. Considering the cost is expected to be negligible and that the code is now more correct, we add and use functions, which should also reduce e2e flakes. Signed-off-by: Francesco Romani --- pkg/deployer/api/api.go | 32 +++++++++++++++++++++---- pkg/deployer/wait/crd.go | 49 +++++++++++++++++++++++++++++++++++++++ pkg/objectwait/api/api.go | 18 ++++++++++++-- 3 files changed, 93 insertions(+), 6 deletions(-) create mode 100644 pkg/deployer/wait/crd.go diff --git a/pkg/deployer/api/api.go b/pkg/deployer/api/api.go index 4b84b608..72b2904f 100644 --- a/pkg/deployer/api/api.go +++ b/pkg/deployer/api/api.go @@ -24,6 +24,7 @@ import ( "github.com/k8stopologyawareschedwg/deployer/pkg/deployer" "github.com/k8stopologyawareschedwg/deployer/pkg/deployer/platform" apimanifests "github.com/k8stopologyawareschedwg/deployer/pkg/manifests/api" + apiwait "github.com/k8stopologyawareschedwg/deployer/pkg/objectwait/api" ) type Options struct { @@ -45,8 +46,19 @@ func Deploy(env *deployer.Environment, opts Options) error { } env.Log.V(3).Info("API manifests loaded") - if err = env.CreateObject(mf.Crd); err != nil { - return err + for _, wo := range apiwait.Creatable(mf, env.Cli, env.Log) { + if err := env.CreateObject(wo.Obj); err != nil { + return err + } + + if wo.Wait == nil { + continue + } + + err = wo.Wait(env.Ctx) + if err != nil { + return err + } } env.Log.Info("deployed topology-aware-scheduling API") @@ -64,8 +76,20 @@ func Remove(env *deployer.Environment, opts Options) error { } env.Log.V(3).Info("API manifests loaded") - if err = env.DeleteObject(mf.Crd); err != nil { - return err + for _, wo := range apiwait.Deletable(mf, env.Cli, env.Log) { + err = env.DeleteObject(wo.Obj) + if err != nil { + continue + } + + if wo.Wait == nil { + continue + } + + err = wo.Wait(env.Ctx) + if err != nil { + env.Log.Info("failed to wait for removal", "error", err) + } } env.Log.Info("removed topology-aware-scheduling API!") diff --git a/pkg/deployer/wait/crd.go b/pkg/deployer/wait/crd.go new file mode 100644 index 00000000..e3adab64 --- /dev/null +++ b/pkg/deployer/wait/crd.go @@ -0,0 +1,49 @@ +/* + * Copyright 2023 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package wait + +import ( + "context" + + apiextensionv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + k8swait "k8s.io/apimachinery/pkg/util/wait" +) + +func (wt Waiter) ForCRDCreated(ctx context.Context, name string) (*apiextensionv1.CustomResourceDefinition, error) { + key := ObjectKey{Name: name} + crd := &apiextensionv1.CustomResourceDefinition{} + err := k8swait.PollImmediate(wt.PollInterval, wt.PollTimeout, func() (bool, error) { + err := wt.Cli.Get(ctx, key.AsKey(), crd) + if err != nil { + wt.Log.Info("failed to get the CRD", "key", key.String(), "error", err) + return false, err + } + + wt.Log.Info("CRD available", "key", key.String()) + return true, nil + }) + return crd, err +} + +func (wt Waiter) ForCRDDeleted(ctx context.Context, name string) error { + return k8swait.PollImmediate(wt.PollInterval, wt.PollTimeout, func() (bool, error) { + obj := apiextensionv1.CustomResourceDefinition{} + key := ObjectKey{Name: name} + err := wt.Cli.Get(ctx, key.AsKey(), &obj) + return deletionStatusFromError(wt.Log, "CRD", key, err) + }) +} diff --git a/pkg/objectwait/api/api.go b/pkg/objectwait/api/api.go index eddb4700..79dfce8e 100644 --- a/pkg/objectwait/api/api.go +++ b/pkg/objectwait/api/api.go @@ -17,22 +17,36 @@ package api import ( + "context" + "github.com/go-logr/logr" "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/k8stopologyawareschedwg/deployer/pkg/deployer/wait" apimf "github.com/k8stopologyawareschedwg/deployer/pkg/manifests/api" "github.com/k8stopologyawareschedwg/deployer/pkg/objectwait" ) func Creatable(mf apimf.Manifests, cli client.Client, log logr.Logger) []objectwait.WaitableObject { return []objectwait.WaitableObject{ - {Obj: mf.Crd}, + { + Obj: mf.Crd, + Wait: func(ctx context.Context) error { + _, err := wait.With(cli, log).ForCRDCreated(ctx, mf.Crd.Name) + return err + }, + }, } } func Deletable(mf apimf.Manifests, cli client.Client, log logr.Logger) []objectwait.WaitableObject { return []objectwait.WaitableObject{ - {Obj: mf.Crd}, + { + Obj: mf.Crd, + Wait: func(ctx context.Context) error { + return wait.With(cli, log).ForCRDDeleted(ctx, mf.Crd.Name) + }, + }, } } From 09d339bd1ed858ec13d0471dfc0061e677636834 Mon Sep 17 00:00:00 2001 From: Francesco Romani Date: Mon, 23 Oct 2023 10:39:13 +0200 Subject: [PATCH 2/2] wait: use consistent approach reactor the code to wait consistently; the previous difference was unwarranted. No intended changes in behavior. Signed-off-by: Francesco Romani --- pkg/deployer/sched/sched.go | 13 ++++++++----- pkg/deployer/updaters/updaters.go | 13 ++++++++----- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/pkg/deployer/sched/sched.go b/pkg/deployer/sched/sched.go index 893ae906..1652dc4f 100644 --- a/pkg/deployer/sched/sched.go +++ b/pkg/deployer/sched/sched.go @@ -69,11 +69,14 @@ func Deploy(env *deployer.Environment, opts Options) error { if err := env.CreateObject(wo.Obj); err != nil { return err } - if opts.WaitCompletion && wo.Wait != nil { - err = wo.Wait(env.Ctx) - if err != nil { - return err - } + + if !opts.WaitCompletion || wo.Wait == nil { + continue + } + + err = wo.Wait(env.Ctx) + if err != nil { + return err } } diff --git a/pkg/deployer/updaters/updaters.go b/pkg/deployer/updaters/updaters.go index eef8524b..b948a563 100644 --- a/pkg/deployer/updaters/updaters.go +++ b/pkg/deployer/updaters/updaters.go @@ -66,11 +66,14 @@ func Deploy(env *deployer.Environment, updaterType string, opts Options) error { if err := env.CreateObject(wo.Obj); err != nil { return err } - if opts.WaitCompletion && wo.Wait != nil { - err = wo.Wait(env.Ctx) - if err != nil { - return err - } + + if !opts.WaitCompletion || wo.Wait == nil { + continue + } + + err = wo.Wait(env.Ctx) + if err != nil { + return err } }