diff --git a/pkg/analyzer/hpa.go b/pkg/analyzer/hpa.go index c720dc1f0f..70b3a5096b 100644 --- a/pkg/analyzer/hpa.go +++ b/pkg/analyzer/hpa.go @@ -5,6 +5,8 @@ import ( "github.com/k8sgpt-ai/k8sgpt/pkg/common" "github.com/k8sgpt-ai/k8sgpt/pkg/util" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -30,28 +32,28 @@ func (HpaAnalyzer) Analyze(a common.Analyzer) ([]common.Result, error) { // check ScaleTargetRef exist scaleTargetRef := hpa.Spec.ScaleTargetRef - scaleTargetRefNotFound := false + var podInfo PodInfo switch scaleTargetRef.Kind { case "Deployment": - _, err := a.Client.GetClient().AppsV1().Deployments(hpa.Namespace).Get(a.Context, scaleTargetRef.Name, metav1.GetOptions{}) - if err != nil { - scaleTargetRefNotFound = true + deployment, err := a.Client.GetClient().AppsV1().Deployments(hpa.Namespace).Get(a.Context, scaleTargetRef.Name, metav1.GetOptions{}) + if err == nil { + podInfo = DeploymentInfo{deployment} } case "ReplicationController": - _, err := a.Client.GetClient().CoreV1().ReplicationControllers(hpa.Namespace).Get(a.Context, scaleTargetRef.Name, metav1.GetOptions{}) - if err != nil { - scaleTargetRefNotFound = true + rc, err := a.Client.GetClient().CoreV1().ReplicationControllers(hpa.Namespace).Get(a.Context, scaleTargetRef.Name, metav1.GetOptions{}) + if err == nil { + podInfo = ReplicationControllerInfo{rc} } case "ReplicaSet": - _, err := a.Client.GetClient().AppsV1().ReplicaSets(hpa.Namespace).Get(a.Context, scaleTargetRef.Name, metav1.GetOptions{}) - if err != nil { - scaleTargetRefNotFound = true + rs, err := a.Client.GetClient().AppsV1().ReplicaSets(hpa.Namespace).Get(a.Context, scaleTargetRef.Name, metav1.GetOptions{}) + if err == nil { + podInfo = ReplicaSetInfo{rs} } case "StatefulSet": - _, err := a.Client.GetClient().AppsV1().StatefulSets(hpa.Namespace).Get(a.Context, scaleTargetRef.Name, metav1.GetOptions{}) - if err != nil { - scaleTargetRefNotFound = true + ss, err := a.Client.GetClient().AppsV1().StatefulSets(hpa.Namespace).Get(a.Context, scaleTargetRef.Name, metav1.GetOptions{}) + if err == nil { + podInfo = StatefulSetInfo{ss} } default: failures = append(failures, common.Failure{ @@ -60,7 +62,7 @@ func (HpaAnalyzer) Analyze(a common.Analyzer) ([]common.Result, error) { }) } - if scaleTargetRefNotFound { + if podInfo == nil { failures = append(failures, common.Failure{ Text: fmt.Sprintf("HorizontalPodAutoscaler uses %s/%s as ScaleTargetRef which does not exist.", scaleTargetRef.Kind, scaleTargetRef.Name), Sensitive: []common.Sensitive{ @@ -70,6 +72,26 @@ func (HpaAnalyzer) Analyze(a common.Analyzer) ([]common.Result, error) { }, }, }) + } else { + containers := len(podInfo.GetPodSpec().Containers) + for _, container := range podInfo.GetPodSpec().Containers { + if container.Resources.Requests == nil || container.Resources.Limits == nil { + containers-- + } + } + + if containers <= 0 { + failures = append(failures, common.Failure{ + Text: fmt.Sprintf("%s %s/%s does not have resource configured.", scaleTargetRef.Kind, a.Namespace, scaleTargetRef.Name), + Sensitive: []common.Sensitive{ + { + Unmasked: scaleTargetRef.Name, + Masked: util.MaskString(scaleTargetRef.Name), + }, + }, + }) + } + } if len(failures) > 0 { @@ -96,3 +118,43 @@ func (HpaAnalyzer) Analyze(a common.Analyzer) ([]common.Result, error) { return a.Results, nil } + +type PodInfo interface { + GetPodSpec() corev1.PodSpec +} + +type DeploymentInfo struct { + *appsv1.Deployment +} + +func (d DeploymentInfo) GetPodSpec() corev1.PodSpec { + return d.Spec.Template.Spec +} + +// define a structure for ReplicationController +type ReplicationControllerInfo struct { + *corev1.ReplicationController +} + +func (rc ReplicationControllerInfo) GetPodSpec() corev1.PodSpec { + return rc.Spec.Template.Spec +} + +// define a structure for ReplicaSet +type ReplicaSetInfo struct { + *appsv1.ReplicaSet +} + +func (rs ReplicaSetInfo) GetPodSpec() corev1.PodSpec { + return rs.Spec.Template.Spec +} + +// define a structure for StatefulSet +type StatefulSetInfo struct { + *appsv1.StatefulSet +} + +// implement PodInfo for StatefulSetInfo +func (ss StatefulSetInfo) GetPodSpec() corev1.PodSpec { + return ss.Spec.Template.Spec +} diff --git a/pkg/analyzer/hpaAnalyzer_test.go b/pkg/analyzer/hpaAnalyzer_test.go index e5d7e32d82..23e7c2c5c1 100644 --- a/pkg/analyzer/hpaAnalyzer_test.go +++ b/pkg/analyzer/hpaAnalyzer_test.go @@ -10,6 +10,8 @@ import ( "github.com/magiconair/properties/assert" appsv1 "k8s.io/api/apps/v1" autoscalingv1 "k8s.io/api/autoscaling/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/fake" ) @@ -163,7 +165,7 @@ func TestHPAAnalyzerWithNonExistentScaleTargetRef(t *testing.T) { } } -func TestHPAAnalyzerWithExistingScaleTargetRef(t *testing.T) { +func TestHPAAnalyzerWithExistingScaleTargetRefAsDeployment(t *testing.T) { clientset := fake.NewSimpleClientset( &autoscalingv1.HorizontalPodAutoscaler{ @@ -185,6 +187,28 @@ func TestHPAAnalyzerWithExistingScaleTargetRef(t *testing.T) { Namespace: "default", Annotations: map[string]string{}, }, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "example", + Image: "nginx", + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + "cpu": resource.MustParse("100m"), + "memory": resource.MustParse("128Mi"), + }, + Limits: corev1.ResourceList{ + "cpu": resource.MustParse("200m"), + "memory": resource.MustParse("256Mi"), + }, + }, + }, + }, + }, + }, + }, }, ) hpaAnalyzer := HpaAnalyzer{} @@ -205,6 +229,265 @@ func TestHPAAnalyzerWithExistingScaleTargetRef(t *testing.T) { } } +func TestHPAAnalyzerWithExistingScaleTargetRefAsReplicationController(t *testing.T) { + + clientset := fake.NewSimpleClientset( + &autoscalingv1.HorizontalPodAutoscaler{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example", + Namespace: "default", + Annotations: map[string]string{}, + }, + Spec: autoscalingv1.HorizontalPodAutoscalerSpec{ + ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{ + Kind: "ReplicationController", + Name: "example", + }, + }, + }, + &corev1.ReplicationController{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example", + Namespace: "default", + Annotations: map[string]string{}, + }, + Spec: corev1.ReplicationControllerSpec{ + Template: &corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "example", + Image: "nginx", + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + "cpu": resource.MustParse("100m"), + "memory": resource.MustParse("128Mi"), + }, + Limits: corev1.ResourceList{ + "cpu": resource.MustParse("200m"), + "memory": resource.MustParse("256Mi"), + }, + }, + }, + }, + }, + }, + }, + }, + ) + hpaAnalyzer := HpaAnalyzer{} + + config := common.Analyzer{ + Client: &kubernetes.Client{ + Client: clientset, + }, + Context: context.Background(), + Namespace: "default", + } + analysisResults, err := hpaAnalyzer.Analyze(config) + if err != nil { + t.Error(err) + } + for _, analysis := range analysisResults { + assert.Equal(t, len(analysis.Error), 0) + } +} + +func TestHPAAnalyzerWithExistingScaleTargetRefAsReplicaSet(t *testing.T) { + + clientset := fake.NewSimpleClientset( + &autoscalingv1.HorizontalPodAutoscaler{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example", + Namespace: "default", + Annotations: map[string]string{}, + }, + Spec: autoscalingv1.HorizontalPodAutoscalerSpec{ + ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{ + Kind: "ReplicaSet", + Name: "example", + }, + }, + }, + &appsv1.ReplicaSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example", + Namespace: "default", + Annotations: map[string]string{}, + }, + Spec: appsv1.ReplicaSetSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "example", + Image: "nginx", + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + "cpu": resource.MustParse("100m"), + "memory": resource.MustParse("128Mi"), + }, + Limits: corev1.ResourceList{ + "cpu": resource.MustParse("200m"), + "memory": resource.MustParse("256Mi"), + }, + }, + }, + }, + }, + }, + }, + }, + ) + hpaAnalyzer := HpaAnalyzer{} + + config := common.Analyzer{ + Client: &kubernetes.Client{ + Client: clientset, + }, + Context: context.Background(), + Namespace: "default", + } + analysisResults, err := hpaAnalyzer.Analyze(config) + if err != nil { + t.Error(err) + } + for _, analysis := range analysisResults { + assert.Equal(t, len(analysis.Error), 0) + } +} + +func TestHPAAnalyzerWithExistingScaleTargetRefAsStatefulSet(t *testing.T) { + + clientset := fake.NewSimpleClientset( + &autoscalingv1.HorizontalPodAutoscaler{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example", + Namespace: "default", + Annotations: map[string]string{}, + }, + Spec: autoscalingv1.HorizontalPodAutoscalerSpec{ + ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{ + Kind: "StatefulSet", + Name: "example", + }, + }, + }, + &appsv1.StatefulSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example", + Namespace: "default", + Annotations: map[string]string{}, + }, + Spec: appsv1.StatefulSetSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "example", + Image: "nginx", + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + "cpu": resource.MustParse("100m"), + "memory": resource.MustParse("128Mi"), + }, + Limits: corev1.ResourceList{ + "cpu": resource.MustParse("200m"), + "memory": resource.MustParse("256Mi"), + }, + }, + }, + }, + }, + }, + }, + }, + ) + hpaAnalyzer := HpaAnalyzer{} + + config := common.Analyzer{ + Client: &kubernetes.Client{ + Client: clientset, + }, + Context: context.Background(), + Namespace: "default", + } + analysisResults, err := hpaAnalyzer.Analyze(config) + if err != nil { + t.Error(err) + } + for _, analysis := range analysisResults { + assert.Equal(t, len(analysis.Error), 0) + } +} + +func TestHPAAnalyzerWithExistingScaleTargetRefWithoutSpecifyingResources(t *testing.T) { + + clientset := fake.NewSimpleClientset( + &autoscalingv1.HorizontalPodAutoscaler{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example", + Namespace: "default", + Annotations: map[string]string{}, + }, + Spec: autoscalingv1.HorizontalPodAutoscalerSpec{ + ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{ + Kind: "Deployment", + Name: "example", + }, + }, + }, + &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example", + Namespace: "default", + Annotations: map[string]string{}, + }, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "example", + Image: "nginx", + }, + }, + }, + }, + }, + }, + ) + hpaAnalyzer := HpaAnalyzer{} + + config := common.Analyzer{ + Client: &kubernetes.Client{ + Client: clientset, + }, + Context: context.Background(), + Namespace: "default", + } + analysisResults, err := hpaAnalyzer.Analyze(config) + if err != nil { + t.Error(err) + } + + var errorFound bool + for _, analysis := range analysisResults { + for _, err := range analysis.Error { + if strings.Contains(err.Text, "does not have resource configured."){ + errorFound = true + break + } + if errorFound { + break + } + } + if !errorFound { + t.Error("expected error 'does not have resource configured.' not found in analysis results") + } + } +} + func TestHPAAnalyzerNamespaceFiltering(t *testing.T) { clientset := fake.NewSimpleClientset( &autoscalingv1.HorizontalPodAutoscaler{