Skip to content

Commit

Permalink
[PLAY-761] correct replicas if HPA present
Browse files Browse the repository at this point in the history
  • Loading branch information
ghkadim committed Feb 9, 2024
1 parent 4ca8b68 commit 9af2025
Show file tree
Hide file tree
Showing 9 changed files with 620 additions and 34 deletions.
10 changes: 8 additions & 2 deletions internal/api/v1beta1/app_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"time"

"github.com/go-logr/logr"
autoscalingv2 "k8s.io/api/autoscaling/v2"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/tools/record"

Expand Down Expand Up @@ -562,7 +563,7 @@ func getUpdatedUnits(weight uint8, targetUnits uint16) (int, int) {

// DoCanary checks if canary deployment is needed for an app and gradually increases the traffic weight
// based on the canary parameters provided by the users. Use it in app controller.
func (app *App) DoCanary(now metav1.Time, logger logr.Logger, recorder record.EventRecorder, disableScaleForProcess map[string]bool) error {
func (app *App) DoCanary(now metav1.Time, logger logr.Logger, recorder record.EventRecorder, disableScaleForProcess map[string]autoscalingv2.HorizontalPodAutoscaler) error {
if !app.Spec.Canary.Active {
failEvent := newCanaryEvent(app, CanaryNotActiveEvent, CanaryNotActiveEventDesc)
recorder.AnnotatedEventf(app, failEvent.Annotations, v1.EventTypeNormal, failEvent.Name, failEvent.Message())
Expand Down Expand Up @@ -596,7 +597,8 @@ func (app *App) DoCanary(now metav1.Time, logger logr.Logger, recorder record.Ev
if app.Spec.Canary.Target != nil {
// scale units based on weight and process target
for processName, target := range app.Spec.Canary.Target {
if _, disable := disableScaleForProcess[processName]; !disable {
deplName := MakeDeploymentName(app.Name, processName, app.Spec.Deployments[1].Version)
if _, disable := disableScaleForProcess[deplName]; !disable {
p1Units, p2Units := getUpdatedUnits(app.Spec.Deployments[0].RoutingSettings.Weight, target)
// might be fine to ignore these errors
if err := app.Spec.Deployments[0].setUnits(processName, p1Units); err != nil {
Expand Down Expand Up @@ -689,6 +691,10 @@ func (app *App) AddLabel(label map[string]string, target Target) {
}
}

func MakeDeploymentName(appName, processName string, deploymentVersion DeploymentVersion) string {
return fmt.Sprintf("%s-%s-%s", appName, processName, deploymentVersion)
}

// PodState describes the simplified state of a pod in the cluster
type PodState string

Expand Down
14 changes: 12 additions & 2 deletions internal/api/v1beta1/app_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/go-logr/logr"
"github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/require"
autoscalingv2 "k8s.io/api/autoscaling/v2"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/tools/record"
Expand Down Expand Up @@ -943,7 +944,7 @@ func TestApp_DoCanary(t *testing.T) {
name string
app App
now metav1.Time
disableScaling map[string]bool
disableScaling map[string]autoscalingv2.HorizontalPodAutoscaler
wantNoChanges bool
wantApp App
wantErr string
Expand Down Expand Up @@ -1166,6 +1167,9 @@ func TestApp_DoCanary(t *testing.T) {
name: "disable pod scaling per process",
now: *timeRef(10, 31),
app: App{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
},
Spec: AppSpec{
Canary: CanarySpec{
Steps: 5,
Expand All @@ -1182,8 +1186,14 @@ func TestApp_DoCanary(t *testing.T) {
},
},
},
disableScaling: map[string]bool{"p1": true},
disableScaling: map[string]autoscalingv2.HorizontalPodAutoscaler{
"test-p1-2": {},
"test-p1-3": {},
},
wantApp: App{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
},
Spec: AppSpec{
Canary: CanarySpec{
Steps: 5,
Expand Down
20 changes: 19 additions & 1 deletion internal/chart/application_chart.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"time"

"helm.sh/helm/v3/pkg/chartutil"
autoscalingv2 "k8s.io/api/autoscaling/v2"
v1 "k8s.io/api/core/v1"
"sigs.k8s.io/yaml"

Expand Down Expand Up @@ -81,6 +82,7 @@ type Options struct {
// ExposedPorts are ports exposed by an image of each deployment.
ExposedPorts map[ketchv1.DeploymentVersion][]ketchv1.ExposedPort
Templates templates.Templates
HPAMap map[string]autoscalingv2.HorizontalPodAutoscaler
}

func WithExposedPorts(ports map[ketchv1.DeploymentVersion][]ketchv1.ExposedPort) Option {
Expand All @@ -102,6 +104,12 @@ func WithTemplates(tpls templates.Templates) Option {
}
}

func WithHPAMap(hpaMap map[string]autoscalingv2.HorizontalPodAutoscaler) Option {
return func(opts *Options) {
opts.HPAMap = hpaMap
}
}

func imagePullSecrets(deploymentImagePullSecrets []v1.LocalObjectReference, spec ketchv1.DockerRegistrySpec) []v1.LocalObjectReference {
if len(deploymentImagePullSecrets) > 0 {
// imagePullSecrets defined for this particular deployment is higher priority.
Expand Down Expand Up @@ -171,7 +179,7 @@ func New(application *ketchv1.App, opts ...Option) (*ApplicationChart, error) {
for _, processSpec := range deploymentSpec.Processes {
name := processSpec.Name
isRoutable := procfile.IsRoutable(name)
process, err := newProcess(name, isRoutable,
processOptions := []processOption{
withCmd(c.procfile.Processes[name]),
withUnits(processSpec.Units),
withEnvs(processSpec.Env),
Expand All @@ -183,6 +191,16 @@ func New(application *ketchv1.App, opts ...Option) (*ApplicationChart, error) {
withVolumeMounts(processSpec.VolumeMounts),
withLabels(application.Spec.Labels, deployment.Version),
withAnnotations(application.Spec.Annotations, deployment.Version),
}
if options.HPAMap != nil {
deplName := ketchv1.MakeDeploymentName(application.Name, processSpec.Name, deployment.Version)
if hpa, ok := options.HPAMap[deplName]; ok {
processOptions = append(processOptions, withHPACurrentReplicas(int(hpa.Status.CurrentReplicas)))
}
}

process, err := newProcess(name, isRoutable,
processOptions...,
)
if err != nil {
return nil, err
Expand Down
25 changes: 25 additions & 0 deletions internal/chart/application_chart_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"helm.sh/helm/v3/pkg/kube/fake"
"helm.sh/helm/v3/pkg/storage"
"helm.sh/helm/v3/pkg/storage/driver"
autoscalingv2 "k8s.io/api/autoscaling/v2"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -39,6 +40,19 @@ func TestNewApplicationChart(t *testing.T) {
ingressControllerWithoutClusterIssuer := ketchv1.IngressControllerSpec{ClassName: "gke",
ServiceEndpoint: "20.20.20.20"}

hpaMap := map[string]autoscalingv2.HorizontalPodAutoscaler{
"dashboard-web-3": {
Status: autoscalingv2.HorizontalPodAutoscalerStatus{
CurrentReplicas: 10,
},
},
"dashboard-worker-4": {
Status: autoscalingv2.HorizontalPodAutoscalerStatus{
CurrentReplicas: 11,
},
},
}

dashboard := &ketchv1.App{
ObjectMeta: metav1.ObjectMeta{
Name: "dashboard",
Expand Down Expand Up @@ -264,6 +278,17 @@ func TestNewApplicationChart(t *testing.T) {
ingressController: ingressControllerWithoutClusterIssuer,
wantYamlsFilename: "dashboard-nginx",
},
{
name: "nginx templates with HPA",
opts: []Option{
WithTemplates(templates.NginxDefaultTemplates),
WithExposedPorts(exportedPorts),
WithHPAMap(hpaMap),
},
application: setServiceAccount(convertSecureEndpoints(dashboard)),
ingressController: ingressControllerWithoutClusterIssuer,
wantYamlsFilename: "dashboard-nginx-hpa",
},
{
name: "istio templates with cluster issuer",
opts: []Option{
Expand Down
24 changes: 16 additions & 8 deletions internal/chart/process.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,15 @@ var (
)

type process struct {
Name string `json:"name"`
Cmd []string `json:"cmd"`
Units int `json:"units"`
Routable bool `json:"routable"`
ContainerPorts []v1.ContainerPort `json:"containerPorts"`
ServicePorts []v1.ServicePort `json:"servicePorts"`
PublicServicePort int32 `json:"publicServicePort,omitempty"`
Env []ketchv1.Env `json:"env"`
Name string `json:"name"`
Cmd []string `json:"cmd"`
Units int `json:"units"`
Routable bool `json:"routable"`
ContainerPorts []v1.ContainerPort `json:"containerPorts"`
ServicePorts []v1.ServicePort `json:"servicePorts"`
PublicServicePort int32 `json:"publicServicePort,omitempty"`
Env []ketchv1.Env `json:"env"`
HPACurrentReplicas int `json:"hpaCurrentReplicas,omitempty"`

SecurityContext *v1.SecurityContext `json:"securityContext,omitempty"`
ResourceRequirements *v1.ResourceRequirements `json:"resourceRequirements,omitempty"`
Expand Down Expand Up @@ -57,6 +58,13 @@ func withUnits(units *int) processOption {
}
}

func withHPACurrentReplicas(count int) processOption {
return func(p *process) error {
p.HPACurrentReplicas = count
return nil
}
}

// withEnvs configures env variables of a process.
// Additionally, the process will have port-related envs like "PORT". Check out "portEnvVariables" below.
func withEnvs(envs []ketchv1.Env) processOption {
Expand Down
Loading

0 comments on commit 9af2025

Please sign in to comment.