Skip to content

Commit

Permalink
Review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
lukasfrank committed Jul 12, 2023
2 parents e9044af + a748b5d commit 68ce29e
Show file tree
Hide file tree
Showing 9 changed files with 339 additions and 25 deletions.
8 changes: 8 additions & 0 deletions poollet/machinepoollet/cmd/machinepoollet/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,14 @@ func Run(ctx context.Context, opts Options) error {
return fmt.Errorf("error setting up machine pool reconciler with manager: %w", err)
}

if err := (&controllers.MachinePoolAnnotatorReconciler{
Client: mgr.GetClient(),
MachinePoolName: opts.MachinePoolName,
MachineClassMapper: machineClassMapper,
}).SetupWithManager(mgr); err != nil {
return fmt.Errorf("error setting up machine pool annotator reconciler with manager: %w", err)
}

return nil
}

Expand Down
14 changes: 10 additions & 4 deletions poollet/machinepoollet/controllers/controllers_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,9 @@ func SetupTest() (*corev1.Namespace, *computev1alpha1.MachinePool, *computev1alp
Expect(machinepoolletclient.SetupNATGatewayRoutingNetworkRefNameField(ctx, indexer)).To(Succeed())
Expect(machinepoolletclient.SetupMachineMachinePoolRefNameField(ctx, indexer)).To(Succeed())

machineClassMapper := mcm.NewGeneric(srv, mcm.GenericOptions{})
machineClassMapper := mcm.NewGeneric(srv, mcm.GenericOptions{
RelistPeriod: 2 * time.Second,
})
Expect(k8sManager.Add(machineClassMapper)).To(Succeed())

mgrCtx, cancel := context.WithCancel(context.Background())
Expand Down Expand Up @@ -247,11 +249,15 @@ func SetupTest() (*corev1.Namespace, *computev1alpha1.MachinePool, *computev1alp

Expect((&controllers.MachinePoolReconciler{
Client: k8sManager.GetClient(),
MachinePoolName: TestMachinePool,
Addresses: []computev1alpha1.MachinePoolAddress{},
Port: 8080,
MachineRuntime: srv,
MachineClassMapper: machineClassMapper,
MachinePoolName: mp.Name,
}).SetupWithManager(k8sManager)).To(Succeed())

Expect((&controllers.MachinePoolAnnotatorReconciler{
Client: k8sManager.GetClient(),
MachineClassMapper: machineClassMapper,
MachinePoolName: mp.Name,
}).SetupWithManager(k8sManager)).To(Succeed())

go func() {
Expand Down
11 changes: 11 additions & 0 deletions poollet/machinepoollet/controllers/machinepool_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
ori "github.com/onmetal/onmetal-api/ori/apis/machine/v1alpha1"
machinepoolletclient "github.com/onmetal/onmetal-api/poollet/machinepoollet/client"
"github.com/onmetal/onmetal-api/poollet/machinepoollet/mcm"
onmetalapiclient "github.com/onmetal/onmetal-api/utils/client"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
ctrl "sigs.k8s.io/controller-runtime"
Expand Down Expand Up @@ -147,6 +148,16 @@ func (r *MachinePoolReconciler) updateResources(log logr.Logger, status *compute
func (r *MachinePoolReconciler) reconcile(ctx context.Context, log logr.Logger, machinePool *computev1alpha1.MachinePool) (ctrl.Result, error) {
log.V(1).Info("Reconcile")

log.V(1).Info("Ensuring no reconcile annotation")
modified, err := onmetalapiclient.PatchEnsureNoReconcileAnnotation(ctx, r.Client, machinePool)
if err != nil {
return ctrl.Result{}, fmt.Errorf("error ensuring no reconcile annotation: %w", err)
}
if modified {
log.V(1).Info("Removed reconcile annotation, requeueing")
return ctrl.Result{Requeue: true}, nil
}

log.V(1).Info("Listing machine classes")
machineClassList := &computev1alpha1.MachineClassList{}
if err := r.List(ctx, machineClassList); err != nil {
Expand Down
114 changes: 94 additions & 20 deletions poollet/machinepoollet/controllers/machinepool_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,10 @@
package controllers_test

import (
"time"

computev1alpha1 "github.com/onmetal/onmetal-api/api/compute/v1alpha1"
corev1alpha1 "github.com/onmetal/onmetal-api/api/core/v1alpha1"
ori "github.com/onmetal/onmetal-api/ori/apis/machine/v1alpha1"
"github.com/onmetal/onmetal-api/ori/testing/machine"
testingmachine "github.com/onmetal/onmetal-api/ori/testing/machine"
"github.com/onmetal/onmetal-api/utils/quota"
. "github.com/onsi/ginkgo/v2"
Expand All @@ -30,12 +29,8 @@ import (
. "sigs.k8s.io/controller-runtime/pkg/envtest/komega"
)

const (
TestMachinePool = "test-machinepool"
)

var _ = Describe("MachinePoolController", func() {
ns, _, mc, srv := SetupTest()
ns, mp, mc, srv := SetupTest()

It("should calculate pool capacity", func(ctx SpecContext) {
var (
Expand All @@ -53,14 +48,6 @@ var _ = Describe("MachinePoolController", func() {
StaticMemory: staticMemory.AsDec().UnscaledBig().Uint64(),
})

By("creating a machine pool")
machinePool := &computev1alpha1.MachinePool{
ObjectMeta: metav1.ObjectMeta{
Name: TestMachinePool,
},
}
Expect(k8sClient.Create(ctx, machinePool)).To(Succeed(), "failed to create machine pool")

By("creating a second machine class")
mc2 := &computev1alpha1.MachineClass{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -96,7 +83,7 @@ var _ = Describe("MachinePoolController", func() {
})

By("checking if the capacity is correct")
Eventually(Object(machinePool)).Within(time.Second).Should(SatisfyAll(
Eventually(Object(mp)).Should(SatisfyAll(
HaveField("Status.Capacity", Satisfy(func(capacity corev1alpha1.ResourceList) bool {
return quota.Equals(capacity, corev1alpha1.ResourceList{
corev1alpha1.SharedResourceCPU: sharedCpu,
Expand All @@ -118,14 +105,14 @@ var _ = Describe("MachinePoolController", func() {
Name: mc2.Name,
},
MachinePoolRef: &corev1.LocalObjectReference{
Name: machinePool.Name,
Name: mp.Name,
},
},
}
Expect(k8sClient.Create(ctx, machine)).To(Succeed(), "failed to create machine")

By("checking if the allocatable resources are correct")
Eventually(Object(machinePool)).Within(time.Second).Should(SatisfyAll(
Eventually(Object(mp)).Should(SatisfyAll(
HaveField("Status.Allocatable", Satisfy(func(allocatable corev1alpha1.ResourceList) bool {
return quota.Equals(allocatable, corev1alpha1.ResourceList{
corev1alpha1.SharedResourceCPU: resource.MustParse("8"),
Expand All @@ -147,14 +134,14 @@ var _ = Describe("MachinePoolController", func() {
Name: mc.Name,
},
MachinePoolRef: &corev1.LocalObjectReference{
Name: machinePool.Name,
Name: mp.Name,
},
},
}
Expect(k8sClient.Create(ctx, machine2)).To(Succeed(), "failed to create test machine class")

By("checking if the allocatable resources are correct")
Eventually(Object(machinePool)).Within(time.Second).Should(SatisfyAll(
Eventually(Object(mp)).Should(SatisfyAll(
HaveField("Status.Allocatable", Satisfy(func(allocatable corev1alpha1.ResourceList) bool {
return quota.Equals(allocatable, corev1alpha1.ResourceList{
corev1alpha1.SharedResourceCPU: resource.MustParse("8"),
Expand All @@ -165,4 +152,91 @@ var _ = Describe("MachinePoolController", func() {
})),
))
})

It("should add machine classes to pool", func(ctx SpecContext) {
srv.SetMachineClasses([]*machine.FakeMachineClass{})

By("creating a machine class")
mc := &computev1alpha1.MachineClass{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "test-mc-1-",
},
Capabilities: corev1alpha1.ResourceList{
corev1alpha1.ResourceCPU: resource.MustParse("2"),
corev1alpha1.ResourceMemory: resource.MustParse("2Gi"),
},
}
Expect(k8sClient.Create(ctx, mc)).To(Succeed(), "failed to create test machine class")

srv.SetMachineClasses([]*machine.FakeMachineClass{
{
MachineClass: ori.MachineClass{
Name: mc.Name,
Capabilities: &ori.MachineClassCapabilities{
CpuMillis: mc.Capabilities.CPU().MilliValue(),
MemoryBytes: mc.Capabilities.Memory().AsDec().UnscaledBig().Uint64(),
},
},
},
})

By("checking if the default machine class is present")
Eventually(Object(mp)).Should(SatisfyAll(
HaveField("Status.AvailableMachineClasses", Equal([]corev1.LocalObjectReference{
{
Name: mc.Name,
},
}))),
)

By("creating a second machine class")
mc2 := &computev1alpha1.MachineClass{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "test-mc-2-",
},
Capabilities: corev1alpha1.ResourceList{
corev1alpha1.ResourceCPU: resource.MustParse("3"),
corev1alpha1.ResourceMemory: resource.MustParse("4Gi"),
},
}
Expect(k8sClient.Create(ctx, mc2)).To(Succeed(), "failed to create test machine class")

Eventually(Object(mp)).Should(SatisfyAll(
HaveField("Status.AvailableMachineClasses", HaveLen(1))),
)

srv.SetMachineClasses([]*machine.FakeMachineClass{
{
MachineClass: ori.MachineClass{
Name: mc.Name,
Capabilities: &ori.MachineClassCapabilities{
CpuMillis: mc.Capabilities.CPU().MilliValue(),
MemoryBytes: mc.Capabilities.Memory().AsDec().UnscaledBig().Uint64(),
},
},
},
{
MachineClass: ori.MachineClass{
Name: mc2.Name,
Capabilities: &ori.MachineClassCapabilities{
CpuMillis: mc2.Capabilities.CPU().MilliValue(),
MemoryBytes: mc2.Capabilities.Memory().AsDec().UnscaledBig().Uint64(),
},
},
},
})

By("checking if the second machine class is present")
Eventually(Object(mp)).Should(SatisfyAll(
HaveField("Status.AvailableMachineClasses", ConsistOf(
corev1.LocalObjectReference{
Name: mc.Name,
},
corev1.LocalObjectReference{
Name: mc2.Name,
},
))),
)
})

})
131 changes: 131 additions & 0 deletions poollet/machinepoollet/controllers/machinepoolannotator_controller.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
// Copyright 2022 OnMetal authors
//
// 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 controllers

import (
"context"
"fmt"

"github.com/go-logr/logr"
computev1alpha1 "github.com/onmetal/onmetal-api/api/compute/v1alpha1"
"github.com/onmetal/onmetal-api/poollet/machinepoollet/mcm"
"github.com/onmetal/onmetal-api/poollet/orievent"
onmetalapiclient "github.com/onmetal/onmetal-api/utils/client"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/event"
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/source"
)

type MachinePoolAnnotatorReconciler struct {
client.Client

MachinePoolName string
MachineClassMapper mcm.MachineClassMapper
}

func (r *MachinePoolAnnotatorReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
machinePool := &computev1alpha1.MachinePool{
ObjectMeta: metav1.ObjectMeta{
Name: req.Name,
},
}

if err := onmetalapiclient.PatchAddReconcileAnnotation(ctx, r.Client, machinePool); client.IgnoreNotFound(err) != nil {
return ctrl.Result{}, fmt.Errorf("error patching machine pool: %w", err)
}
return ctrl.Result{}, nil
}

func (r *MachinePoolAnnotatorReconciler) SetupWithManager(mgr ctrl.Manager) error {
c, err := controller.New("machinepoolannotator", mgr, controller.Options{
Reconciler: r,
})
if err != nil {
return err
}

src, err := r.oriClassEventSource(mgr)
if err != nil {
return err
}

if err := c.Watch(src, handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, obj client.Object) []ctrl.Request {
return []ctrl.Request{{NamespacedName: client.ObjectKey{Name: r.MachinePoolName}}}
})); err != nil {
return err
}

return nil
}

func (r *MachinePoolAnnotatorReconciler) machinePoolAnnotatorEventHandler(log logr.Logger, c chan<- event.GenericEvent) orievent.EnqueueFunc {
handleEvent := func() {
select {
case c <- event.GenericEvent{Object: &computev1alpha1.MachinePool{ObjectMeta: metav1.ObjectMeta{
Name: r.MachinePoolName,
}}}:
log.V(1).Info("Added item to queue")
default:
log.V(5).Info("Channel full, discarding event")
}
}

return orievent.EnqueueFunc{EnqueueFunc: handleEvent}
}

func (r *MachinePoolAnnotatorReconciler) oriClassEventSource(mgr ctrl.Manager) (source.Source, error) {
ch := make(chan event.GenericEvent, 1024)

if err := mgr.Add(manager.RunnableFunc(func(ctx context.Context) error {
log := ctrl.LoggerFrom(ctx).WithName("machinepool").WithName("orieventhandlers")

notifierFuncs := []func() (orievent.ListenerRegistration, error){
func() (orievent.ListenerRegistration, error) {
return r.MachineClassMapper.AddListener(r.machinePoolAnnotatorEventHandler(log, ch))
},
}

var notifier []orievent.ListenerRegistration
defer func() {
log.V(1).Info("Removing notifier")
for _, n := range notifier {
if err := r.MachineClassMapper.RemoveListener(n); err != nil {
log.Error(err, "Error removing handle")
}
}
}()

for _, notifierFunc := range notifierFuncs {
ntf, err := notifierFunc()
if err != nil {
return err
}

notifier = append(notifier, ntf)
}

<-ctx.Done()
return nil
})); err != nil {
return nil, err
}

return &source.Channel{Source: ch}, nil
}
3 changes: 2 additions & 1 deletion poollet/machinepoollet/controllers/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@ package controllers
import (
ori "github.com/onmetal/onmetal-api/ori/apis/machine/v1alpha1"
utilslices "github.com/onmetal/onmetal-api/utils/slices"
"golang.org/x/exp/constraints"
)

func Max[T uint64 | int64](x, y T) T {
func Max[T constraints.Ordered](x, y T) T {
if x < y {
return y
}
Expand Down
Loading

0 comments on commit 68ce29e

Please sign in to comment.