Skip to content

Commit

Permalink
Merge pull request #7413 from adrianmoisey/emit_event_on_vpa
Browse files Browse the repository at this point in the history 
VPA: Create event for VPA object when Pod is evicted
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Nov 6, 2024
2 parents 9e15df8 + 8137019 commit 9c91496
Show file tree
Hide file tree
Showing 5 changed files with 221 additions and 29 deletions.
7 changes: 5 additions & 2 deletions vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ const (
type PodsEvictionRestriction interface {
// Evict sends eviction instruction to the api client.
// Returns error if pod cannot be evicted or if client returned error.
Evict(pod *apiv1.Pod, eventRecorder record.EventRecorder) error
Evict(pod *apiv1.Pod, vpa *vpa_types.VerticalPodAutoscaler, eventRecorder record.EventRecorder) error
// CanEvict checks if pod can be safely evicted
CanEvict(pod *apiv1.Pod) bool
}
Expand Down Expand Up @@ -122,7 +122,7 @@ func (e *podsEvictionRestrictionImpl) CanEvict(pod *apiv1.Pod) bool {

// Evict sends eviction instruction to api client. Returns error if pod cannot be evicted or if client returned error
// Does not check if pod was actually evicted after eviction grace period.
func (e *podsEvictionRestrictionImpl) Evict(podToEvict *apiv1.Pod, eventRecorder record.EventRecorder) error {
func (e *podsEvictionRestrictionImpl) Evict(podToEvict *apiv1.Pod, vpa *vpa_types.VerticalPodAutoscaler, eventRecorder record.EventRecorder) error {
cr, present := e.podToReplicaCreatorMap[getPodID(podToEvict)]
if !present {
return fmt.Errorf("pod not suitable for eviction %s/%s: not in replicated pods map", podToEvict.Namespace, podToEvict.Name)
Expand All @@ -146,6 +146,9 @@ func (e *podsEvictionRestrictionImpl) Evict(podToEvict *apiv1.Pod, eventRecorder
eventRecorder.Event(podToEvict, apiv1.EventTypeNormal, "EvictedByVPA",
"Pod was evicted by VPA Updater to apply resource recommendation.")

eventRecorder.Event(vpa, apiv1.EventTypeNormal, "EvictedPod",
"VPA Updater evicted Pod "+podToEvict.Name+" to apply resource recommendation.")

if podToEvict.Status.Phase != apiv1.PodPending {
singleGroupStats, present := e.creatorToSingleGroupStatsMap[cr]
if !present {
Expand Down
141 changes: 119 additions & 22 deletions vertical-pod-autoscaler/pkg/updater/eviction/pods_eviction_restriction_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import (
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"

appsv1 "k8s.io/api/apps/v1"
batchv1 "k8s.io/api/batch/v1"
apiv1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -281,15 +283,14 @@ func TestEvictReplicatedByController(t *testing.T) {
assert.Equalf(t, p.canEvict, eviction.CanEvict(p.pod), "TC %v - unexpected CanEvict result for pod-%v %#v", testCase.name, i, p.pod)
}
for i, p := range testCase.pods {
err := eviction.Evict(p.pod, test.FakeEventRecorder())
err := eviction.Evict(p.pod, testCase.vpa, test.FakeEventRecorder())
if p.evictionSuccess {
assert.NoErrorf(t, err, "TC %v - unexpected Evict result for pod-%v %#v", testCase.name, i, p.pod)
} else {
assert.Errorf(t, err, "TC %v - unexpected Evict result for pod-%v %#v", testCase.name, i, p.pod)
}
}
}

}

func TestEvictReplicatedByReplicaSet(t *testing.T) {
Expand All @@ -314,19 +315,20 @@ func TestEvictReplicatedByReplicaSet(t *testing.T) {
pods[i] = test.Pod().WithName(getTestPodName(i)).WithCreator(&rs.ObjectMeta, &rs.TypeMeta).Get()
}

basicVpa := getBasicVpa()
factory, _ := getEvictionRestrictionFactory(nil, &rs, nil, nil, 2, 0.5)
eviction := factory.NewPodsEvictionRestriction(pods, getBasicVpa())
eviction := factory.NewPodsEvictionRestriction(pods, basicVpa)

for _, pod := range pods {
assert.True(t, eviction.CanEvict(pod))
}

for _, pod := range pods[:2] {
err := eviction.Evict(pod, test.FakeEventRecorder())
err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder())
assert.Nil(t, err, "Should evict with no error")
}
for _, pod := range pods[2:] {
err := eviction.Evict(pod, test.FakeEventRecorder())
err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder())
assert.Error(t, err, "Error expected")
}
}
Expand All @@ -353,19 +355,20 @@ func TestEvictReplicatedByStatefulSet(t *testing.T) {
pods[i] = test.Pod().WithName(getTestPodName(i)).WithCreator(&ss.ObjectMeta, &ss.TypeMeta).Get()
}

basicVpa := getBasicVpa()
factory, _ := getEvictionRestrictionFactory(nil, nil, &ss, nil, 2, 0.5)
eviction := factory.NewPodsEvictionRestriction(pods, getBasicVpa())
eviction := factory.NewPodsEvictionRestriction(pods, basicVpa)

for _, pod := range pods {
assert.True(t, eviction.CanEvict(pod))
}

for _, pod := range pods[:2] {
err := eviction.Evict(pod, test.FakeEventRecorder())
err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder())
assert.Nil(t, err, "Should evict with no error")
}
for _, pod := range pods[2:] {
err := eviction.Evict(pod, test.FakeEventRecorder())
err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder())
assert.Error(t, err, "Error expected")
}
}
Expand All @@ -390,19 +393,21 @@ func TestEvictReplicatedByDaemonSet(t *testing.T) {
for i := range pods {
pods[i] = test.Pod().WithName(getTestPodName(i)).WithCreator(&ds.ObjectMeta, &ds.TypeMeta).Get()
}

basicVpa := getBasicVpa()
factory, _ := getEvictionRestrictionFactory(nil, nil, nil, &ds, 2, 0.5)
eviction := factory.NewPodsEvictionRestriction(pods, getBasicVpa())
eviction := factory.NewPodsEvictionRestriction(pods, basicVpa)

for _, pod := range pods {
assert.True(t, eviction.CanEvict(pod))
}

for _, pod := range pods[:2] {
err := eviction.Evict(pod, test.FakeEventRecorder())
err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder())
assert.Nil(t, err, "Should evict with no error")
}
for _, pod := range pods[2:] {
err := eviction.Evict(pod, test.FakeEventRecorder())
err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder())
assert.Error(t, err, "Error expected")
}
}
Expand All @@ -425,19 +430,20 @@ func TestEvictReplicatedByJob(t *testing.T) {
pods[i] = test.Pod().WithName(getTestPodName(i)).WithCreator(&job.ObjectMeta, &job.TypeMeta).Get()
}

basicVpa := getBasicVpa()
factory, _ := getEvictionRestrictionFactory(nil, nil, nil, nil, 2, 0.5)
eviction := factory.NewPodsEvictionRestriction(pods, getBasicVpa())
eviction := factory.NewPodsEvictionRestriction(pods, basicVpa)

for _, pod := range pods {
assert.True(t, eviction.CanEvict(pod))
}

for _, pod := range pods[:2] {
err := eviction.Evict(pod, test.FakeEventRecorder())
err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder())
assert.Nil(t, err, "Should evict with no error")
}
for _, pod := range pods[2:] {
err := eviction.Evict(pod, test.FakeEventRecorder())
err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder())
assert.Error(t, err, "Error expected")
}
}
Expand All @@ -464,15 +470,16 @@ func TestEvictTooFewReplicas(t *testing.T) {
pods[i] = test.Pod().WithName(getTestPodName(i)).WithCreator(&rc.ObjectMeta, &rc.TypeMeta).Get()
}

basicVpa := getBasicVpa()
factory, _ := getEvictionRestrictionFactory(&rc, nil, nil, nil, 10, 0.5)
eviction := factory.NewPodsEvictionRestriction(pods, getBasicVpa())
eviction := factory.NewPodsEvictionRestriction(pods, basicVpa)

for _, pod := range pods {
assert.False(t, eviction.CanEvict(pod))
}

for _, pod := range pods {
err := eviction.Evict(pod, test.FakeEventRecorder())
err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder())
assert.Error(t, err, "Error expected")
}
}
Expand Down Expand Up @@ -500,19 +507,20 @@ func TestEvictionTolerance(t *testing.T) {
pods[i] = test.Pod().WithName(getTestPodName(i)).WithCreator(&rc.ObjectMeta, &rc.TypeMeta).Get()
}

basicVpa := getBasicVpa()
factory, _ := getEvictionRestrictionFactory(&rc, nil, nil, nil, 2 /*minReplicas*/, tolerance)
eviction := factory.NewPodsEvictionRestriction(pods, getBasicVpa())
eviction := factory.NewPodsEvictionRestriction(pods, basicVpa)

for _, pod := range pods {
assert.True(t, eviction.CanEvict(pod))
}

for _, pod := range pods[:4] {
err := eviction.Evict(pod, test.FakeEventRecorder())
err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder())
assert.Nil(t, err, "Should evict with no error")
}
for _, pod := range pods[4:] {
err := eviction.Evict(pod, test.FakeEventRecorder())
err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder())
assert.Error(t, err, "Error expected")
}
}
Expand Down Expand Up @@ -540,23 +548,112 @@ func TestEvictAtLeastOne(t *testing.T) {
pods[i] = test.Pod().WithName(getTestPodName(i)).WithCreator(&rc.ObjectMeta, &rc.TypeMeta).Get()
}

basicVpa := getBasicVpa()
factory, _ := getEvictionRestrictionFactory(&rc, nil, nil, nil, 2, tolerance)
eviction := factory.NewPodsEvictionRestriction(pods, getBasicVpa())
eviction := factory.NewPodsEvictionRestriction(pods, basicVpa)

for _, pod := range pods {
assert.True(t, eviction.CanEvict(pod))
}

for _, pod := range pods[:1] {
err := eviction.Evict(pod, test.FakeEventRecorder())
err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder())
assert.Nil(t, err, "Should evict with no error")
}
for _, pod := range pods[1:] {
err := eviction.Evict(pod, test.FakeEventRecorder())
err := eviction.Evict(pod, basicVpa, test.FakeEventRecorder())
assert.Error(t, err, "Error expected")
}
}

func TestEvictEmitEvent(t *testing.T) {
rc := apiv1.ReplicationController{
ObjectMeta: metav1.ObjectMeta{
Name: "rc",
Namespace: "default",
},
TypeMeta: metav1.TypeMeta{
Kind: "ReplicationController",
},
}

index := 0
generatePod := func() test.PodBuilder {
index++
return test.Pod().WithName(fmt.Sprintf("test-%v", index)).WithCreator(&rc.ObjectMeta, &rc.TypeMeta)
}

basicVpa := getBasicVpa()

testCases := []struct {
name string
replicas int32
evictionTolerance float64
vpa *vpa_types.VerticalPodAutoscaler
pods []podWithExpectations
}{
{
name: "Pods that can be evicted",
replicas: 4,
evictionTolerance: 0.5,
vpa: basicVpa,
pods: []podWithExpectations{
{
pod: generatePod().WithPhase(apiv1.PodPending).Get(),
canEvict: true,
evictionSuccess: true,
},
{
pod: generatePod().WithPhase(apiv1.PodPending).Get(),
canEvict: true,
evictionSuccess: true,
},
},
},
{
name: "Pod that can not be evicted",
replicas: 4,
evictionTolerance: 0.5,
vpa: basicVpa,
pods: []podWithExpectations{

{
pod: generatePod().Get(),
canEvict: false,
evictionSuccess: false,
},
},
},
}

for _, testCase := range testCases {
rc.Spec = apiv1.ReplicationControllerSpec{
Replicas: &testCase.replicas,
}
pods := make([]*apiv1.Pod, 0, len(testCase.pods))
for _, p := range testCase.pods {
pods = append(pods, p.pod)
}
factory, _ := getEvictionRestrictionFactory(&rc, nil, nil, nil, 2, testCase.evictionTolerance)
eviction := factory.NewPodsEvictionRestriction(pods, testCase.vpa)

for _, p := range testCase.pods {
mockRecorder := test.MockEventRecorder()
mockRecorder.On("Event", mock.Anything, apiv1.EventTypeNormal, "EvictedByVPA", mock.Anything).Return()
mockRecorder.On("Event", mock.Anything, apiv1.EventTypeNormal, "EvictedPod", mock.Anything).Return()

eviction.Evict(p.pod, testCase.vpa, mockRecorder)

if p.canEvict {
mockRecorder.AssertNumberOfCalls(t, "Event", 2)

} else {
mockRecorder.AssertNumberOfCalls(t, "Event", 0)
}
}
}
}

func getEvictionRestrictionFactory(rc *apiv1.ReplicationController, rs *appsv1.ReplicaSet,
ss *appsv1.StatefulSet, ds *appsv1.DaemonSet, minReplicas int,
evictionToleranceFraction float64) (PodsEvictionRestrictionFactory, error) {
Expand Down
16 changes: 13 additions & 3 deletions vertical-pod-autoscaler/pkg/updater/logic/updater.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ import (
"k8s.io/apimachinery/pkg/labels"
kube_client "k8s.io/client-go/kubernetes"
"k8s.io/client-go/kubernetes/fake"
"k8s.io/client-go/kubernetes/scheme"

corescheme "k8s.io/client-go/kubernetes/scheme"
clientv1 "k8s.io/client-go/kubernetes/typed/core/v1"
v1lister "k8s.io/client-go/listers/core/v1"
"k8s.io/client-go/tools/cache"
Expand All @@ -38,6 +39,7 @@ import (

vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1"
vpa_clientset "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/client/clientset/versioned"
"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/client/clientset/versioned/scheme"
vpa_lister "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/client/listers/autoscaling.k8s.io/v1"
"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/target"
controllerfetcher "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/target/controller_fetcher"
Expand Down Expand Up @@ -225,7 +227,7 @@ func (u *updater) RunOnce(ctx context.Context) {
return
}
klog.V(2).InfoS("Evicting pod", "pod", klog.KObj(pod))
evictErr := evictionLimiter.Evict(pod, u.eventRecorder)
evictErr := evictionLimiter.Evict(pod, vpa, u.eventRecorder)
if evictErr != nil {
klog.V(0).InfoS("Eviction failed", "error", evictErr, "pod", klog.KObj(pod))
} else {
Expand Down Expand Up @@ -310,6 +312,14 @@ func newEventRecorder(kubeClient kube_client.Interface) record.EventRecorder {
eventBroadcaster.StartLogging(klog.V(4).InfoS)
if _, isFake := kubeClient.(*fake.Clientset); !isFake {
eventBroadcaster.StartRecordingToSink(&clientv1.EventSinkImpl{Interface: clientv1.New(kubeClient.CoreV1().RESTClient()).Events("")})
} else {
eventBroadcaster.StartRecordingToSink(&clientv1.EventSinkImpl{Interface: kubeClient.CoreV1().Events("")})
}
return eventBroadcaster.NewRecorder(scheme.Scheme, apiv1.EventSource{Component: "vpa-updater"})

vpascheme := scheme.Scheme
if err := corescheme.AddToScheme(vpascheme); err != nil {
klog.Fatalf("Error adding core scheme: %v", err)
}

return eventBroadcaster.NewRecorder(vpascheme, apiv1.EventSource{Component: "vpa-updater"})
}
Loading

0 comments on commit 9c91496

Please sign in to comment.