Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: improve HPA analyzer to check ScaleTargetRef resources #283

Merged
merged 9 commits into from
Apr 18, 2023
Merged

feat: improve HPA analyzer to check ScaleTargetRef resources #283

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d1a8931
feat: improve HPA analyzer to check ScaleTargetRef resources
rakshitgondwal Apr 15, 2023
62e3fb0
feat: modify tests
rakshitgondwal Apr 15, 2023
0ceed62
Merge branch 'k8sgpt-ai:main' into dev
rakshitgondwal Apr 17, 2023
3150b1b
feat: improve all ScaleTargetRef to check for resources
rakshitgondwal Apr 17, 2023
fc62ac6
test: add test cases for all ScaleTargetRef types
rakshitgondwal Apr 17, 2023
af06ef1
refactor: use interface to avoid dupplication
matthisholleville Apr 17, 2023
b8dcccc
Merge branch 'main' into dev
matthisholleville Apr 17, 2023
fc607cd
Merge branch 'k8sgpt-ai:main' into dev
rakshitgondwal Apr 18, 2023
970b6f7
test: add test case for NoResourceConfiguredForScaleTargetRef
rakshitgondwal Apr 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
60 changes: 56 additions & 4 deletions pkg/analyzer/hpa.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,24 +34,76 @@ func (HpaAnalyzer) Analyze(a common.Analyzer) ([]common.Result, error) {

switch scaleTargetRef.Kind {
case "Deployment":
_, err := a.Client.GetClient().AppsV1().Deployments(hpa.Namespace).Get(a.Context, scaleTargetRef.Name, metav1.GetOptions{})
deployment, err := a.Client.GetClient().AppsV1().Deployments(hpa.Namespace).Get(a.Context, scaleTargetRef.Name, metav1.GetOptions{})
if err != nil {
scaleTargetRefNotFound = true
} else {
// check if the deployment has resource configured
if (deployment.Spec.Template.Spec.Containers[0].Resources.Requests == nil) || (deployment.Spec.Template.Spec.Containers[0].Resources.Limits == nil) {
failures = append(failures, common.Failure{
Text: fmt.Sprintf("Deployment %s/%s does not have resource configured.", deployment.Namespace, deployment.Name),
Sensitive: []common.Sensitive{
{
Unmasked: deployment.Name,
Masked: util.MaskString(deployment.Name),
},
},
})
}
}
case "ReplicationController":
_, err := a.Client.GetClient().CoreV1().ReplicationControllers(hpa.Namespace).Get(a.Context, scaleTargetRef.Name, metav1.GetOptions{})
rc, err := a.Client.GetClient().CoreV1().ReplicationControllers(hpa.Namespace).Get(a.Context, scaleTargetRef.Name, metav1.GetOptions{})
if err != nil {
scaleTargetRefNotFound = true
} else {
matthisholleville marked this conversation as resolved.
Show resolved Hide resolved
// check if the replication controller has resource configured
if (rc.Spec.Template.Spec.Containers[0].Resources.Requests == nil) || (rc.Spec.Template.Spec.Containers[0].Resources.Limits == nil) {
failures = append(failures, common.Failure{
Text: fmt.Sprintf("ReplicationController %s/%s does not have resource configured.", rc.Namespace, rc.Name),
Sensitive: []common.Sensitive{
{
Unmasked: rc.Name,
Masked: util.MaskString(rc.Name),
},
},
})
}
}
case "ReplicaSet":
_, err := a.Client.GetClient().AppsV1().ReplicaSets(hpa.Namespace).Get(a.Context, scaleTargetRef.Name, metav1.GetOptions{})
rs, err := a.Client.GetClient().AppsV1().ReplicaSets(hpa.Namespace).Get(a.Context, scaleTargetRef.Name, metav1.GetOptions{})
if err != nil {
scaleTargetRefNotFound = true
} else {
// check if the replica set has resource configured
if (rs.Spec.Template.Spec.Containers[0].Resources.Requests == nil) || (rs.Spec.Template.Spec.Containers[0].Resources.Limits == nil) {
failures = append(failures, common.Failure{
Text: fmt.Sprintf("ReplicaSet %s/%s does not have resource configured.", rs.Namespace, rs.Name),
Sensitive: []common.Sensitive{
{
Unmasked: rs.Name,
Masked: util.MaskString(rs.Name),
},
},
})
}
}
case "StatefulSet":
_, err := a.Client.GetClient().AppsV1().StatefulSets(hpa.Namespace).Get(a.Context, scaleTargetRef.Name, metav1.GetOptions{})
ss, err := a.Client.GetClient().AppsV1().StatefulSets(hpa.Namespace).Get(a.Context, scaleTargetRef.Name, metav1.GetOptions{})
if err != nil {
scaleTargetRefNotFound = true
} else {
// check if the stateful set has resource configured
if (ss.Spec.Template.Spec.Containers[0].Resources.Requests == nil) || (ss.Spec.Template.Spec.Containers[0].Resources.Limits == nil) {
failures = append(failures, common.Failure{
Text: fmt.Sprintf("StatefulSet %s/%s does not have resource configured.", ss.Namespace, ss.Name),
Sensitive: []common.Sensitive{
{
Unmasked: ss.Name,
Masked: util.MaskString(ss.Name),
},
},
})
}
}
default:
failures = append(failures, common.Failure{
Expand Down
218 changes: 217 additions & 1 deletion pkg/analyzer/hpaAnalyzer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -163,7 +165,7 @@ func TestHPAAnalyzerWithNonExistentScaleTargetRef(t *testing.T) {
}
}

func TestHPAAnalyzerWithExistingScaleTargetRef(t *testing.T) {
func TestHPAAnalyzerWithExistingScaleTargetRefAsDeployment(t *testing.T) {

clientset := fake.NewSimpleClientset(
&autoscalingv1.HorizontalPodAutoscaler{
Expand All @@ -185,6 +187,220 @@ 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{}

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 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"),
},
},
},
},
},
},
},
},
matthisholleville marked this conversation as resolved.
Show resolved Hide resolved
)
hpaAnalyzer := HpaAnalyzer{}
Expand Down