Skip to content

Commit

Permalink
feat: use deployment replica check from kubescore
Browse files Browse the repository at this point in the history
  • Loading branch information
ReuDa committed Jan 16, 2024
1 parent 1061841 commit 234a5f4
Show file tree
Hide file tree
Showing 9 changed files with 131 additions and 6 deletions.
2 changes: 1 addition & 1 deletion charts/steadybit-extension-kubernetes/Chart.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
apiVersion: v2
name: steadybit-extension-kubernetes
description: Steadybit Kubernetes extension Helm chart for Kubernetes.
version: 1.4.49
version: 1.4.50
appVersion: latest
home: https://www.steadybit.com/
icon: https://steadybit-website-assets.s3.amazonaws.com/logo-symbol-transparent.png
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,15 @@ rules:
- get
- list
- watch
{{/* Required for Single-Replica-Advice */}}
- apiGroups:
- autoscaling
resources:
- horizontalpodautoscalers
verbs:
- get
- list
- watch
{{/* Required for Rollout Restart Attack */}}
- apiGroups:
- apps
Expand Down
34 changes: 34 additions & 0 deletions client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/rs/zerolog/log"
"golang.org/x/exp/slices"
appsv1 "k8s.io/api/apps/v1"
autoscalingv2 "k8s.io/api/autoscaling/v2"
corev1 "k8s.io/api/core/v1"
k8sErrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -18,6 +19,7 @@ import (
"k8s.io/client-go/informers"
"k8s.io/client-go/kubernetes"
listerAppsv1 "k8s.io/client-go/listers/apps/v1"
listerAutoscalingv2 "k8s.io/client-go/listers/autoscaling/v2"
listerCorev1 "k8s.io/client-go/listers/core/v1"
"k8s.io/client-go/rest"
"k8s.io/client-go/tools/cache"
Expand Down Expand Up @@ -75,6 +77,11 @@ type Client struct {
informer cache.SharedIndexInformer
}

hpa struct {
lister listerAutoscalingv2.HorizontalPodAutoscalerLister
informer cache.SharedIndexInformer
}

handlers struct {
sync.Mutex
l []chan<- interface{}
Expand Down Expand Up @@ -241,6 +248,20 @@ func (c *Client) Events(since time.Time) *[]corev1.Event {
return &result
}

func (c *Client) HorizontalPodAutoscalerByNamespaceAndDeployment(namespace string, reference string) *autoscalingv2.HorizontalPodAutoscaler {
hpas, err := c.hpa.lister.HorizontalPodAutoscalers(namespace).List(labels.Everything())
if err != nil {
log.Error().Err(err).Msgf("Error while fetching horizontal pod autoscalers")
return nil
}
for _, hpa := range hpas {
if hpa.Spec.ScaleTargetRef.Kind == "Deployment" && hpa.Spec.ScaleTargetRef.Name == reference {
return hpa
}
}
return nil
}

func logGetError(resource string, err error) {
if err != nil {
var t *k8sErrors.StatusError
Expand Down Expand Up @@ -368,6 +389,19 @@ func CreateClient(clientset kubernetes.Interface, stopCh <-chan struct{}, rootAp
log.Fatal().Msg("failed to add node event handler")
}

if permissions.CanReadHorizontalPodAutoscalers() {
hpa := factory.Autoscaling().V2().HorizontalPodAutoscalers()
client.hpa.informer = hpa.Informer()
client.hpa.lister = hpa.Lister()
informerSyncList = append(informerSyncList, client.hpa.informer.HasSynced)
if err := client.hpa.informer.SetTransform(transformHPA); err != nil {
log.Fatal().Err(err).Msg("Failed to add hpa transformer")
}
if _, err := client.hpa.informer.AddEventHandler(client.resourceEventHandler); err != nil {
log.Fatal().Msg("failed to add hpa event handler")
}
}

events := factory.Core().V1().Events()
client.event.informer = events.Informer()
informerSyncList = append(informerSyncList, client.event.informer.HasSynced)
Expand Down
8 changes: 8 additions & 0 deletions client/permissions.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ var requiredPermissions = []requiredPermission{
{group: "apps", resource: "replicasets", verbs: []string{"get", "list", "watch"}, allowGracefulFailure: false},
{group: "apps", resource: "daemonsets", verbs: []string{"get", "list", "watch"}, allowGracefulFailure: false},
{group: "apps", resource: "statefulsets", verbs: []string{"get", "list", "watch"}, allowGracefulFailure: false},
{group: "autoscaling", resource: "horizontalpodautoscalers", verbs: []string{"get", "list", "watch"}, allowGracefulFailure: true},
{group: "", resource: "services", verbs: []string{"get", "list", "watch"}, allowGracefulFailure: false},
{group: "", resource: "pods", verbs: []string{"get", "list", "watch"}, allowGracefulFailure: false},
{group: "", resource: "nodes", verbs: []string{"get", "list", "watch"}, allowGracefulFailure: false},
Expand Down Expand Up @@ -133,6 +134,13 @@ func (p *PermissionCheckResult) hasPermissions(requiredPermissions []string) boo
return true
}

func (p *PermissionCheckResult) CanReadHorizontalPodAutoscalers() bool {
return p.hasPermissions([]string{
"autoscaling/horizontalpodautoscalers/get",
"autoscaling/horizontalpodautoscalers/list",
"autoscaling/horizontalpodautoscalers/watch"})
}

func (p *PermissionCheckResult) IsRolloutRestartPermitted() bool {
return p.hasPermissions([]string{
"apps/deployments/patch",
Expand Down
6 changes: 3 additions & 3 deletions extadvice/advice.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,11 +210,11 @@ func GetAdviceDescriptionSingleReplica() advice_kit_api.AdviceDefinition {
Label: "Redundant Pod Deployment",
Version: extbuild.GetSemverVersionStringOrUnknown(),
Icon: "data:image/svg+xml,%3Csvg%20width%3D%2224%22%20height%3D%2224%22%20viewBox%3D%220%200%2024%2024%22%20fill%3D%22none%22%20xmlns%3D%22http%3A%2F%2Fwww.w3.org%2F2000%2Fsvg%22%3E%0A%3Cpath%20d%3D%22M11.9436%207.04563C12.1262%206.98477%2012.3235%206.98477%2012.5061%207.04563L17.8407%208.82395C18.2037%208.94498%2018.4486%209.28468%2018.4485%209.66728C18.4485%2010.0499%2018.2036%2010.3895%2017.8405%2010.5105L12.5059%2012.2877C12.3235%2012.3485%2012.1262%2012.3485%2011.9438%2012.2877L6.60918%2010.5105C6.24611%2010.3895%206.00119%2010.0499%206.00116%209.66728C6.00112%209.28468%206.24598%208.94498%206.60902%208.82395L11.9436%207.04563Z%22%20fill%3D%22%231D2632%22%2F%3E%0A%3Cpath%20d%3D%22M7.20674%2013.2736C6.68268%2013.0989%206.11622%2013.3821%205.94153%2013.9062C5.76684%2014.4302%206.05007%2014.9967%206.57414%2015.1714L11.9087%2016.9496C12.114%2017.018%2012.336%2017.018%2012.5413%2016.9496L17.8759%2015.1714C18.4%2014.9967%2018.6832%2014.4302%2018.5085%2013.9062C18.3338%2013.3821%2017.7674%2013.0989%2017.2433%2013.2736L12.225%2014.9463L7.20674%2013.2736Z%22%20fill%3D%22%231D2632%22%2F%3E%0A%3Cpath%20fill-rule%3D%22evenodd%22%20clip-rule%3D%22evenodd%22%20d%3D%22M11.6491%201.06354C11.8754%200.97882%2012.1246%200.97882%2012.3509%201.06354L22.3506%204.80836C22.7412%204.95463%2023%205.32784%2023%205.74482V18.2552C23%2018.6722%2022.7412%2019.0454%2022.3506%2019.1916L12.3509%2022.9365C12.1246%2023.0212%2011.8754%2023.0212%2011.6491%2022.9365L1.64938%2019.1916C1.2588%2019.0454%201%2018.6722%201%2018.2552V5.74482C1%205.32784%201.2588%204.95463%201.64938%204.80836L11.6491%201.06354ZM3.00047%206.43809V17.5619L12%2020.9321L20.9995%2017.5619V6.43809L12%203.06785L3.00047%206.43809Z%22%20fill%3D%22%231D2632%22%2F%3E%0A%3C%2Fsvg%3E%0A",
Tags: &[]string{"kubernetes", "deployment", "statefulset", "replica", "pod"},
AssessmentQueryApplicable: "target.type=\"" + extdeployment.DeploymentTargetType + "\" OR target.type=\"" + extstatefulset.StatefulSetTargetType + "\"",
Tags: &[]string{"kubernetes", "deployment", "replica", "pod"},
AssessmentQueryApplicable: "target.type=\"" + extdeployment.DeploymentTargetType + "\" AND k8s.specification.has-multiple-replica IS PRESENT",
Status: advice_kit_api.AdviceDefinitionStatus{
ActionNeeded: advice_kit_api.AdviceDefinitionStatusActionNeeded{
AssessmentQuery: "k8s.specification.replicas IS NOT PRESENT OR k8s.specification.replicas = \"1\"",
AssessmentQuery: "k8s.specification.has-multiple-replica=\"false\"",
Description: advice_kit_api.AdviceDefinitionStatusActionNeededDescription{
Instruction: ReadAdviceFile(SingleReplicaContent, "single_replica/instructions.md"),
Motivation: ReadAdviceFile(SingleReplicaContent, "single_replica/motivation.md"),
Expand Down
3 changes: 3 additions & 0 deletions extadvice/single_replica/instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,6 @@ spec:
tier: ${target.steadybit.label:normal}
```
**If you increase the replica we strongly advice you to check if this is supported by your application.**
---
This advice is powered by [kube-score](https://kube-score.com/).
9 changes: 8 additions & 1 deletion extcommon/kubescore.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/zegl/kube-score/scorecard"
"io"
appsv1 "k8s.io/api/apps/v1"
autoscalingv2 "k8s.io/api/autoscaling/v2"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/runtime"
k8sJson "k8s.io/apimachinery/pkg/runtime/serializer/json"
Expand All @@ -37,7 +38,7 @@ type kubeScoreInput interface {
GetNamespace() string
}

func GetKubeScoreForDeployment(deployment *appsv1.Deployment, services []*corev1.Service) map[string][]string {
func GetKubeScoreForDeployment(deployment *appsv1.Deployment, services []*corev1.Service, hpa *autoscalingv2.HorizontalPodAutoscaler) map[string][]string {
deployment.APIVersion = "apps/v1"
deployment.Kind = "Deployment"
inputs := make([]kubeScoreInput, 0)
Expand All @@ -47,6 +48,11 @@ func GetKubeScoreForDeployment(deployment *appsv1.Deployment, services []*corev1
service.Kind = "Service"
inputs = append(inputs, service)
}
if hpa != nil {
hpa.APIVersion = "autoscaling/v2"
hpa.Kind = "HorizontalPodAutoscaler"
inputs = append(inputs, hpa)
}

attributes := map[string][]string{}

Expand All @@ -58,6 +64,7 @@ func GetKubeScoreForDeployment(deployment *appsv1.Deployment, services []*corev1
addContainerBasedScore(scores, attributes, "container-image-pull-policy", "k8s.container.image.without-image-pull-policy-always")
addSimpleScore(scores, attributes, "deployment-has-host-podantiaffinity", "k8s.specification.has-host-podantiaffinity")
addSimpleScore(scores, attributes, "deployment-strategy", "k8s.specification.has-rolling-update-strategy")
addSimpleScore(scores, attributes, "deployment-replicas", "k8s.specification.has-multiple-replica")

return attributes
}
Expand Down
7 changes: 6 additions & 1 deletion extdeployment/deployment_discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/steadybit/extension-kubernetes/extcommon"
"github.com/steadybit/extension-kubernetes/extconfig"
appsv1 "k8s.io/api/apps/v1"
autoscalingv2 "k8s.io/api/autoscaling/v2"
corev1 "k8s.io/api/core/v1"
"k8s.io/utils/strings/slices"
"reflect"
Expand Down Expand Up @@ -114,7 +115,11 @@ func (d *deploymentDiscovery) DiscoverTargets(_ context.Context) ([]discovery_ki
attributes[key] = value
}

for key, value := range extcommon.GetKubeScoreForDeployment(deployment, d.k8s.ServicesMatchingToPodLabels(deployment.Namespace, deployment.Spec.Template.Labels)) {
var hpa *autoscalingv2.HorizontalPodAutoscaler
if d.k8s.Permissions().CanReadHorizontalPodAutoscalers() {
hpa = d.k8s.HorizontalPodAutoscalerByNamespaceAndDeployment(deployment.Namespace, deployment.Name)
}
for key, value := range extcommon.GetKubeScoreForDeployment(deployment, d.k8s.ServicesMatchingToPodLabels(deployment.Namespace, deployment.Spec.Template.Labels), hpa) {
attributes[key] = value
}

Expand Down
59 changes: 59 additions & 0 deletions extdeployment/deployment_discovery_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,40 @@ func Test_deploymentDiscovery(t *testing.T) {
"k8s.specification.has-rolling-update-strategy": {"true"},
},
},
{
name: "should report single replica",
pods: []*v1.Pod{testPod("aaaaa", nil)},
deployment: testDeployment(func(deployment *appsv1.Deployment) {
deployment.Spec.Replicas = extutil.Ptr(int32(1))
}),
service: testService(nil),
expectedAttributes: map[string][]string{
"k8s.specification.has-multiple-replica": {"false"},
},
},
{
name: "should report multiple replicas",
pods: []*v1.Pod{testPod("aaaaa", nil)},
deployment: testDeployment(nil),
service: testService(nil),
expectedAttributes: map[string][]string{
"k8s.specification.has-multiple-replica": {"true"},
},
},
{
name: "should not report multiple replicas if no service is defined",
pods: []*v1.Pod{testPod("aaaaa", nil)},
deployment: testDeployment(nil),
expectedAttributesAbsence: []string{"k8s.specification.has-multiple-replica"},
},
{
name: "should not report multiple replicas if targeted by hpa",
pods: []*v1.Pod{testPod("aaaaa", nil)},
deployment: testDeployment(nil),
service: testService(nil),
hpa: testHPA(nil),
expectedAttributesAbsence: []string{"k8s.specification.has-multiple-replica"},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down Expand Up @@ -349,6 +383,31 @@ func Test_deploymentDiscovery(t *testing.T) {
}
}

func testHPA(modifier func(autoscaler *autoscalingv2.HorizontalPodAutoscaler)) *autoscalingv2.HorizontalPodAutoscaler {
autoscaler := &autoscalingv2.HorizontalPodAutoscaler{
TypeMeta: metav1.TypeMeta{
Kind: "HorizontalPodAutoscaler",
APIVersion: "autoscaling/v2",
},
ObjectMeta: metav1.ObjectMeta{
Name: "shop",
Namespace: "default",
},
Spec: autoscalingv2.HorizontalPodAutoscalerSpec{
ScaleTargetRef: autoscalingv2.CrossVersionObjectReference{
Kind: "Deployment",
Name: "shop",
APIVersion: "apps/v1",
},
},
}
if modifier != nil {
modifier(autoscaler)
}

return autoscaler
}

func testDeployment(modifier func(*appsv1.Deployment)) *appsv1.Deployment {
deployment := &appsv1.Deployment{
TypeMeta: metav1.TypeMeta{
Expand Down

0 comments on commit 234a5f4

Please sign in to comment.