Skip to content

Commit

Permalink
Apply review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
plkokanov committed Aug 26, 2020
1 parent 383244e commit b8f2510
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 67 deletions.
8 changes: 4 additions & 4 deletions pkg/gardenlet/controller/shoot/shoot_control_reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -405,18 +405,18 @@ func (c *Controller) runReconcileShootFlow(o *operation.Operation) *gardencorev1
})
_ = g.Add(flow.Task{
Name: "Waiting until container runtime resources are ready",
Fn: flow.TaskFn(botanist.Shoot.Components.Extensions.ContainerRuntime.Wait),
Fn: botanist.Shoot.Components.Extensions.ContainerRuntime.Wait,
Dependencies: flow.NewTaskIDs(deployContainerRuntimeResources),
})
deleteStaleContainerRuntimeResources = g.Add(flow.Task{
deleteStaleResources = g.Add(flow.Task{
Name: "Delete stale container runtime resources",
Fn: flow.TaskFn(botanist.Shoot.Components.Extensions.ContainerRuntime.DeleteStaleContainerRuntimeResources).RetryUntilTimeout(defaultInterval, defaultTimeout),
Fn: flow.TaskFn(botanist.Shoot.Components.Extensions.ContainerRuntime.DeleteStaleResources).RetryUntilTimeout(defaultInterval, defaultTimeout),
Dependencies: flow.NewTaskIDs(initializeShootClients),
})
_ = g.Add(flow.Task{
Name: "Waiting until stale container runtime resources are deleted",
Fn: flow.TaskFn(botanist.Shoot.Components.Extensions.ContainerRuntime.WaitCleanup).SkipIf(o.Shoot.HibernationEnabled),
Dependencies: flow.NewTaskIDs(deleteStaleContainerRuntimeResources),
Dependencies: flow.NewTaskIDs(deleteStaleResources),
})
_ = g.Add(flow.Task{
Name: "Restart control plane pods",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ import (
"fmt"
"time"

"github.com/gardener/gardener/pkg/apis/core/v1alpha1"
"github.com/gardener/gardener/pkg/apis/core/v1beta1"
gardencorev1alpha1 "github.com/gardener/gardener/pkg/apis/core/v1alpha1"
gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1"
v1beta1constants "github.com/gardener/gardener/pkg/apis/core/v1beta1/constants"
extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1"
"github.com/gardener/gardener/pkg/operation/common"
Expand All @@ -41,7 +41,7 @@ const (
// DefaultSevereThreshold is the default threshold until an error reported by another component is treated as 'severe'.
DefaultSevereThreshold = 30 * time.Second
// DefaultTimeout is the default timeout and defines how long Gardener should wait
// for a successful reconciliation of a network resource.
// for a successful reconciliation of a containerruntime resource.
DefaultTimeout = 3 * time.Minute
)

Expand All @@ -51,7 +51,7 @@ var TimeNow = time.Now
// Values contains the values used to create a ContainerRuntime resources.
type Values struct {
Namespace string
Workers []v1beta1.Worker
Workers []gardencorev1beta1.Worker
}

type containerruntime struct {
Expand Down Expand Up @@ -80,12 +80,11 @@ func New(
waitSevereThreshold: waitSevereThreshold,
waitTimeout: waitTimeout,
}

}

// Deploy uses the seed client to create or update the ContainerRuntime resources.
func (d *containerruntime) Deploy(ctx context.Context) error {
fns := d.forEachContainerRuntime(func(ctx context.Context, workerName string, cr v1beta1.ContainerRuntime) error {
fns := d.forEachContainerRuntime(func(ctx context.Context, workerName string, cr gardencorev1beta1.ContainerRuntime) error {
rd := resourceDeployer{d.values.Namespace, workerName, cr, d.client}
_, err := rd.deploy(ctx, v1beta1constants.GardenerOperationReconcile)
return err
Expand All @@ -101,7 +100,7 @@ func (d *containerruntime) Destroy(ctx context.Context) error {

// Wait waits until the ContainerRuntime resources are ready.
func (d *containerruntime) Wait(ctx context.Context) error {
fns := d.forEachContainerRuntime(func(ctx context.Context, workerName string, cr v1beta1.ContainerRuntime) error {
fns := d.forEachContainerRuntime(func(ctx context.Context, workerName string, cr gardencorev1beta1.ContainerRuntime) error {
return common.WaitUntilExtensionCRReady(
ctx,
d.client,
Expand Down Expand Up @@ -137,8 +136,8 @@ func (d *containerruntime) WaitCleanup(ctx context.Context) error {
}

// Restore uses the seed client and the ShootState to create the ContainereRuntime resources and restore their state.
func (d *containerruntime) Restore(ctx context.Context, shootState *v1alpha1.ShootState) error {
fns := d.forEachContainerRuntime(func(ctx context.Context, workerName string, cr v1beta1.ContainerRuntime) error {
func (d *containerruntime) Restore(ctx context.Context, shootState *gardencorev1alpha1.ShootState) error {
fns := d.forEachContainerRuntime(func(ctx context.Context, workerName string, cr gardencorev1beta1.ContainerRuntime) error {
rd := resourceDeployer{d.values.Namespace, workerName, cr, d.client}
return common.RestoreExtensionWithDeployFunction(ctx, shootState, d.client, extensionsv1alpha1.ContainerRuntimeResource, d.values.Namespace, rd.deploy)
})
Expand Down Expand Up @@ -170,8 +169,8 @@ func (d *containerruntime) WaitMigrate(ctx context.Context) error {
)
}

// DeleteStaleContainerRuntimeResources deletes unused container runtime resources from the shoot namespace in the seed.
func (d *containerruntime) DeleteStaleContainerRuntimeResources(ctx context.Context) error {
// DeleteStaleResources deletes unused container runtime resources from the shoot namespace in the seed.
func (d *containerruntime) DeleteStaleResources(ctx context.Context) error {
wantedContainerRuntimeTypes := sets.NewString()
for _, worker := range d.values.Workers {
if worker.CRI != nil {
Expand Down Expand Up @@ -201,7 +200,7 @@ func (d *containerruntime) deleteContainerRuntimeResources(ctx context.Context,
)
}

func (d *containerruntime) forEachContainerRuntime(fn func(ctx context.Context, workerName string, cr v1beta1.ContainerRuntime) error) []flow.TaskFn {
func (d *containerruntime) forEachContainerRuntime(fn func(ctx context.Context, workerName string, cr gardencorev1beta1.ContainerRuntime) error) []flow.TaskFn {
var fns []flow.TaskFn
for _, worker := range d.values.Workers {
if worker.CRI == nil {
Expand All @@ -226,7 +225,7 @@ func getContainerRuntimeKey(criType, workerName string) string {
type resourceDeployer struct {
namespace string
workerName string
containerRuntime v1beta1.ContainerRuntime
containerRuntime gardencorev1beta1.ContainerRuntime
client client.Client
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ var _ = Describe("#ContainerRuntimee", func() {
})
})

Describe("#DeleteStaleContainerRuntimeResources", func() {
Describe("#DeleteStaleResources", func() {
It("should delete stale containerruntime resources", func() {
staleContainerRuntime := expected[0].DeepCopy()
staleContainerRuntime.Name = fmt.Sprintf("%s-%s", "new-type", workerNames[0])
Expand All @@ -427,7 +427,7 @@ var _ = Describe("#ContainerRuntimee", func() {
Expect(c.Create(ctx, e)).To(Succeed(), "creating containerruntime succeeds")
}

Expect(defaultDepWaiter.DeleteStaleContainerRuntimeResources(ctx)).To(Succeed())
Expect(defaultDepWaiter.DeleteStaleResources(ctx)).To(Succeed())

containerRuntimeList := &extensionsv1alpha1.ContainerRuntimeList{}
Expect(c.List(ctx, containerRuntimeList)).To(Succeed())
Expand Down
27 changes: 4 additions & 23 deletions pkg/operation/botanist/migration.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,12 @@ import (

// AnnotateExtensionCRsForMigration annotates extension CRs with migrate operation annotation
func (b *Botanist) AnnotateExtensionCRsForMigration(ctx context.Context) (err error) {
if err = b.Shoot.Components.Extensions.Network.Migrate(ctx); err != nil {
return err
}
if err := b.Shoot.Components.Extensions.ContainerRuntime.Migrate(ctx); err != nil {
return err
}

var fns []flow.TaskFn
fns, err = b.applyFuncToAllExtensionCRs(ctx, annotateObjectForMigrationFunc(ctx, b.K8sSeedClient.DirectClient()))
if err != nil {
return err
}
fns = append(fns, b.Shoot.Components.Extensions.Network.Migrate, b.Shoot.Components.Extensions.ContainerRuntime.Migrate)

return flow.Parallel(fns...)(ctx)
}
Expand All @@ -57,14 +51,6 @@ func annotateObjectForMigrationFunc(ctx context.Context, client client.Client) f

// WaitForExtensionsOperationMigrateToSucceed waits until extension CRs has lastOperation Migrate Succeeded
func (b *Botanist) WaitForExtensionsOperationMigrateToSucceed(ctx context.Context) error {
if err := b.Shoot.Components.Extensions.Network.WaitMigrate(ctx); err != nil {
return err
}

if err := b.Shoot.Components.Extensions.ContainerRuntime.WaitMigrate(ctx); err != nil {
return err
}

fns, err := b.applyFuncToAllExtensionCRs(ctx, func(obj runtime.Object) error {
extensionObj, err := extensions.Accessor(obj)
if err != nil {
Expand All @@ -83,20 +69,13 @@ func (b *Botanist) WaitForExtensionsOperationMigrateToSucceed(ctx context.Contex
if err != nil {
return err
}
fns = append(fns, b.Shoot.Components.Extensions.Network.WaitMigrate, b.Shoot.Components.Extensions.ContainerRuntime.WaitMigrate)

return flow.Parallel(fns...)(ctx)
}

// DeleteAllExtensionCRs deletes all extension CRs from the Shoot namespace
func (b *Botanist) DeleteAllExtensionCRs(ctx context.Context) error {
if err := b.Shoot.Components.Extensions.Network.Destroy(ctx); err != nil {
return err
}

if err := b.Shoot.Components.Extensions.ContainerRuntime.Destroy(ctx); err != nil {
return err
}

fns, err := b.applyFuncToAllExtensionCRs(ctx, func(obj runtime.Object) error {
extensionObj, err := extensions.Accessor(obj)
if err != nil {
Expand All @@ -108,6 +87,8 @@ func (b *Botanist) DeleteAllExtensionCRs(ctx context.Context) error {
if err != nil {
return err
}
fns = append(fns, b.Shoot.Components.Extensions.Network.Destroy, b.Shoot.Components.Extensions.ContainerRuntime.Destroy)

return flow.Parallel(fns...)(ctx)
}

Expand Down
74 changes: 53 additions & 21 deletions pkg/operation/botanist/migration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@ package botanist_test

import (
"context"
"fmt"
"time"

"github.com/gardener/gardener/pkg/apis/core/v1beta1"
"github.com/gardener/gardener/pkg/api/extensions"
gardencorev1beta1 "github.com/gardener/gardener/pkg/apis/core/v1beta1"
v1beta1constants "github.com/gardener/gardener/pkg/apis/core/v1beta1/constants"
extensionsv1alpha1 "github.com/gardener/gardener/pkg/apis/extensions/v1alpha1"
extensionsclient "github.com/gardener/gardener/pkg/client/extensions/clientset/versioned/scheme"
Expand All @@ -35,26 +37,51 @@ import (
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
)

var _ = Describe("control plane migration", func() {
const (
testSeedNamespace = "test-seed-namespace"
workerName = "test-worker"
networkName = "test-network"
containerRuntimeName = "testContainerRuntime"
)

var (
ctrl *gomock.Controller
fakeClient client.Client
k8sSeedClient *fakeclientset.ClientSet
testSeedNamespace = "test-seed-namespace"
ctrl *gomock.Controller
fakeClient client.Client
k8sSeedClient *fakeclientset.ClientSet
expected []runtime.Object
)

BeforeEach(func() {
ctrl = gomock.NewController(GinkgoT())
expected = []runtime.Object{
&extensionsv1alpha1.Worker{
ObjectMeta: metav1.ObjectMeta{
Name: workerName,
Namespace: testSeedNamespace,
},
},
&extensionsv1alpha1.Network{
ObjectMeta: metav1.ObjectMeta{
Name: networkName,
Namespace: testSeedNamespace,
},
},
&extensionsv1alpha1.ContainerRuntime{
ObjectMeta: metav1.ObjectMeta{
Name: containerRuntimeName,
Namespace: testSeedNamespace,
},
},
}

fakeClient = fakeclient.NewFakeClientWithScheme(extensionsclient.Scheme, &extensionsv1alpha1.Worker{ObjectMeta: metav1.ObjectMeta{
Name: "test-worker",
Namespace: testSeedNamespace,
}})
fakeClient = fakeclient.NewFakeClientWithScheme(extensionsclient.Scheme, expected...)
k8sSeedClient = fakeclientset.NewClientSetBuilder().WithClient(fakeClient).WithDirectClient(fakeClient).Build()
})

Expand All @@ -72,12 +99,12 @@ var _ = Describe("control plane migration", func() {
Components: &shoot.Components{
Extensions: &shoot.Extensions{
Network: network.New(log, fakeClient, &network.Values{
Namespace: "test-network",
Name: testSeedNamespace,
Namespace: testSeedNamespace,
Name: networkName,
}, time.Second, 2*time.Second, 3*time.Second),
ContainerRuntime: containerruntime.New(log, fakeClient, &containerruntime.Values{
Namespace: "test-containerruntime",
Workers: []v1beta1.Worker{},
Namespace: testSeedNamespace,
Workers: []gardencorev1beta1.Worker{},
}, time.Second, 2*time.Second, 3*time.Second),
},
},
Expand All @@ -93,14 +120,19 @@ var _ = Describe("control plane migration", func() {
err := botanist.AnnotateExtensionCRsForMigration(ctx)
Expect(err).NotTo(HaveOccurred())

actualWorker := &extensionsv1alpha1.Worker{}
err = fakeClient.Get(ctx, types.NamespacedName{
Name: "test-worker",
Namespace: testSeedNamespace,
}, actualWorker)
Expect(err).NotTo(HaveOccurred())
Expect(actualWorker.GetAnnotations()).NotTo(BeNil())
Expect(actualWorker.GetAnnotations()[v1beta1constants.GardenerOperation]).To(Equal(v1beta1constants.GardenerOperationMigrate))
for _, obj := range expected {
actual, err := extensions.Accessor(obj)
Expect(err).NotTo(HaveOccurred())

Expect(
fakeClient.Get(ctx, types.NamespacedName{Name: actual.GetName(), Namespace: testSeedNamespace}, actual),
).To(Succeed())

Expect(actual.GetAnnotations()).NotTo(BeNil(), fmt.Sprintf("%s should have annotations", actual.GetName()))
Expect(
actual.GetAnnotations()[v1beta1constants.GardenerOperation],
).To(Equal(v1beta1constants.GardenerOperationMigrate), fmt.Sprintf("%s should have migrate annotation", actual.GetName()))
}
})
})
})
4 changes: 1 addition & 3 deletions pkg/operation/common/extensions.go
Original file line number Diff line number Diff line change
Expand Up @@ -456,9 +456,7 @@ func MigrateExtensionCRs(
namespace string,
) error {
fns, err := applyFuncToExtensionResources(ctx, c, listObj, namespace, nil, func(ctx context.Context, o extensionsv1alpha1.Object) error {
return MigrateExtensionCR(
ctx, c, newObjFunc, o.GetNamespace(), o.GetName(),
)
return MigrateExtensionCR(ctx, c, newObjFunc, o.GetNamespace(), o.GetName())
})

if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/operation/shoot/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ type Infrastructure interface {
// ContainerRuntime contains references to a ContainerRuntime extension deployer.
type ContainerRuntime interface {
component.DeployMigrateWaiter
DeleteStaleContainerRuntimeResources(ctx context.Context) error
DeleteStaleResources(ctx context.Context) error
}

// Networks contains pre-calculated subnets and IP address for various components.
Expand Down

0 comments on commit b8f2510

Please sign in to comment.