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.

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

Adding 1->n and n->1 scaling for Queue scaler #47

Merged
merged 20 commits into from Mar 27, 2019
Merged

Adding 1->n and n->1 scaling for Queue scaler #47

merged 20 commits into from Mar 27, 2019

Conversation

Aarthisk
Copy link
Contributor

Adding HPA scaling for Queues.
Changes include

  • Creating a HPA when a scaled object is synced
  • Deleting a HPA when a scaled object is deleted
  • Additional logic to not scale to zero immediately
  • Scaler interface changes to get the new metric definitions and metrics
  • Metric adapter that implements Metrics API for HPA to query and scale on external metrics
  • /deploy has the deployment script for the metrics adapter

@yaron2
Copy link
Contributor

yaron2 commented Mar 21, 2019

Excited to see this!

@yaron2
Copy link
Contributor

yaron2 commented Mar 21, 2019

Once #41 is merged, will you need to update based on the cool-down period work?

@ahmelsayed
Copy link
Contributor

btw, I can't build this PR. I thought the CI error was a test error, but looking at it again, the CI is also failing to build. The error is


vendor/github.com/emicklei/go-restful/container.go:17:2: cannot find package "github.com/emicklei/go-restful/log" in any of:
        /home/ahmed/go/src/github.com/Azure/Kore/vendor/github.com/emicklei/go-restful/log (vendor tree)
        /usr/lib/go-1.10/src/github.com/emicklei/go-restful/log (from $GOROOT)
        /home/ahmed/go/src/github.com/emicklei/go-restful/log (from $GOPATH)
vendor/k8s.io/apiserver/pkg/server/options/audit.go:38:2: cannot find package "k8s.io/apiserver/plugin/pkg/audit/log" in any of:
        /home/ahmed/go/src/github.com/Azure/Kore/vendor/k8s.io/apiserver/plugin/pkg/audit/log (vendor tree)
        /usr/lib/go-1.10/src/k8s.io/apiserver/plugin/pkg/audit/log (from $GOROOT)
        /home/ahmed/go/src/k8s.io/apiserver/plugin/pkg/audit/log (from $GOPATH) 

I tried running dep ensure and it's been running for about 10 minutes now.....

@ahmelsayed
Copy link
Contributor

also there is a status.png file that I don't think you meant to commit

.vscode/launch.json Show resolved Hide resolved
cmd/main.go Outdated Show resolved Hide resolved
pkg/controller/controller.go Outdated Show resolved Hide resolved
pkg/handler/scale_handler.go Show resolved Hide resolved
pkg/handler/scale_handler.go Outdated Show resolved Hide resolved
pkg/handler/scale_handler.go Outdated Show resolved Hide resolved
newhpa := &v2beta1.HorizontalPodAutoscaler{Spec: *newHpaSpec, ObjectMeta: *hpaObjectSpec}
newhpa, err = h.kubeClient.AutoscalingV2beta1().HorizontalPodAutoscalers(scaledObjectNamespace).Create(newhpa)
if errors.IsAlreadyExists(err) {
log.Warnf("HPA with namespace %s and name %s already exists", scaledObjectNamespace, hpaName)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we need to have an "upgrade" mechanism once people update kore's version, so that we can change the HPA scaling logic on kore updates

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

possibly- I can see us updating the max replicas when the scaled object overrides it for example.

pkg/handler/scale_handler.go Outdated Show resolved Hide resolved
pkg/provider/provider.go Outdated Show resolved Hide resolved
@Aarthisk
Copy link
Contributor Author

also there is a status.png file that I don't think you meant to commit

I did not. Removed.

@Aarthisk
Copy link
Contributor Author

also there is a status.png file that I don't think you meant to commit

Yes, removed.

@Aarthisk Aarthisk closed this Mar 22, 2019
@Aarthisk Aarthisk reopened this Mar 22, 2019
@Aarthisk
Copy link
Contributor Author

removed

@Aarthisk
Copy link
Contributor Author

Once #41 is merged, will you need to update based on the cool-down period work?

This is now removed.

@@ -39,9 +43,100 @@ func NewScaleHandler(koreClient clientset.Interface, kubeClient kubernetes.Inter

// WatchScaledObjectWithContext enqueues the ScaledObject into the work queue
func (h *ScaleHandler) WatchScaledObjectWithContext(ctx context.Context, scaledObject *kore_v1alpha1.ScaledObject) {
h.createHPAForNewScaledObject(ctx, scaledObject)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be called asynchronously as well i think.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also keep in mind that this is called on every scaledobject update. That can be frequent

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be called asynchronously but I don't think it should.
The WatchScaledObjectWithContext itself is executed on a separate goroutine from the get go, no?

pkg/adapter/adapter.go Outdated Show resolved Hide resolved
@@ -39,9 +43,100 @@ func NewScaleHandler(koreClient clientset.Interface, kubeClient kubernetes.Inter

// WatchScaledObjectWithContext enqueues the ScaledObject into the work queue
func (h *ScaleHandler) WatchScaledObjectWithContext(ctx context.Context, scaledObject *kore_v1alpha1.ScaledObject) {
h.createHPAForNewScaledObject(ctx, scaledObject)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be called asynchronously but I don't think it should.
The WatchScaledObjectWithContext itself is executed on a separate goroutine from the get go, no?

pkg/handler/scale_handler.go Outdated Show resolved Hide resolved
pkg/handler/scale_handler.go Outdated Show resolved Hide resolved
pkg/handler/scale_handler.go Outdated Show resolved Hide resolved
pkg/scalers/azure_queue_scaler.go Outdated Show resolved Hide resolved
@yaron2
Copy link
Contributor

yaron2 commented Mar 26, 2019

Review submitted.

@yaron2 yaron2 mentioned this pull request Mar 26, 2019
@yaron2
Copy link
Contributor

yaron2 commented Mar 27, 2019

Please resolve conflicts.

…work

# Conflicts:
#	.circleci/config.yml
#	pkg/scalers/azure_queue.go
#	pkg/scalers/azure_queue_scaler.go
@yaron2
Copy link
Contributor

yaron2 commented Mar 27, 2019

LGTM

@yaron2 yaron2 merged commit 5e4e37e into master Mar 27, 2019
@yaron2 yaron2 deleted the aarthisk-work branch March 27, 2019 17:41
patnaikshekhar pushed a commit to patnaikshekhar/keda that referenced this pull request Jul 17, 2019
Adding 1->n and n->1 scaling for Queue scaler

Former-commit-id: 5e4e37e
Aarthisk pushed a commit that referenced this pull request Aug 12, 2019
Adding 1->n and n->1 scaling for Queue scaler

Former-commit-id: 0e2b157
preflightsiren pushed a commit to preflightsiren/keda that referenced this pull request Nov 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants