Few updates #6

ahmelsayed · 2019-02-20T22:49:33Z

Add CircleCI (9400db6, fd95193, b4f4f29) https://circleci.com/gh/Azure/Kore (login with your github account)
Create a ScaleHandler object that the controller calls for every ScaledObject
Update ScaledObject OM
This changes the OM to

apiVersion: kore.k8s.io/v1alpha1
kind: ScaledObject
metadata:
  name: _________
spec:
  scaleTargetRef:
    deploymentName: _______
  triggers:
  - type: azure-queue
    metadata:
      _______: _______

which removes secretRef from the ScaledObject object. the ScaleHandler just copies all of the env section to the scalers.

Update Gopkg.toml locking the versions of kubernetes to 1.13.3
Update vendor to include k8s.io/code-generator and k8s.io/gogen
Added instructions for running kore locally then publishing a functions project to your cluster. It should scale if there is an azure queue trigger in the functions project.

CI builds each branch and then pushes the results to

projectkore.azurecr.io/project-kore/kore:{branch-name}

so for example this PR will create projectkore.azurecr.io/project-kore/kore:ahmels-work

There are no tests yet, so the CI isn't running anything. It's just building the project and building a docker image for it and pushing it to ACR projectkore.azurecr.io

ahmelsayed · 2019-02-20T22:52:09Z

cmd/main.go

+	// workaround go dep management system
+	_ "k8s.io/code-generator/pkg/util"
+	_ "k8s.io/gengo/parser"
+	_ "golang.org/x/tools/imports"


these repos are needed in the vendor folder for hack/generate-groups.sh to work for auto-generating the client SDK for our custom ScaledObject types. This was the workaround I found people doing in Go to workaround dep removing referenced dependencies from the vendor folder. I'm not sure if there is a better way.

ahmelsayed · 2019-02-20T22:54:15Z

cmd/main.go

@@ -16,7 +22,8 @@ func main() {
 	}

 	ctx := signals.Context()
-	controller.NewController(koreClient, kubeClient).Run(ctx)
+	scaleHandler := scalers.NewScaleHandler(koreClient, kubeClient)
+	controller.NewController(koreClient, kubeClient, scaleHandler).Run(ctx)



The ScaleHandler is the interface between the Controller and the Adapter on one end, and the individual scalers on the other.

Currently the interface for it is just

HandleScale(scaledObject *kore_v1alpha1.ScaledObject)

This is fine

ahmelsayed · 2019-02-20T22:59:35Z

pkg/controller/controller.go

+			// used for polling external systems like we have with autoscalers.
+			// This however makes it not possible to have a custom check interval
+			// per ScaledObject or deployment.
+			time.Second*30,


I mentioned this in one of the emails. Looking at some online discussion linked above, it seems that resyncPeriod is there for the Informer to playback all of its cache to the UpdateFunc handler. This will simplify things as we will just rely on the Informer cache to playback all the ScaledObjects it has every 30 seconds, and we can check the respective scalers for events.

The alternative is to build our own queue/channel and keep a copy of the Informer cache state ourselves in the ScaleHandler and maintain our own timer for enqueuing the ScaledObject into the work queue. We will also have to handle the DeleteFunc properly with some shared collection that we take a RWLock on, I think. Right?

I don't see a reason to use resync. our requirements are to get updated on every CRD update/added events. I don't see why we would need to replay the cache?

This will just use the cache/resyncPeriod instead of implementing our own store for these ScaledObjects and our own timer for checking them.

Basically we will need to store these ScaledObjects in a shared collection and do proper RWLock for adding, updating, or removing ScaledObjects, then have our own timer to check their trigger status for scale up/down.

Configuring the resyncPeriod shifts that responsibility to the Informer as it'll keep pinging our code every 30 seconds in this example instead of us maintaining our own timer. Would you rather we keep our own copy and timer for checking these triggers?

I'd rather we keep our own copy and checking these triggers. We need to be able to configurably change the interval to accommodate for various latency SLAs

ahmelsayed · 2019-02-20T23:00:16Z

pkg/controller/controller.go

-			}
-
+			// always call syncScaledObject even on resync
+			// this uses the informer cache for updates rather than maintaining another cache
 			c.syncScaledObject(newObj)


make sure to always call syncScaledObject on every UpdateFunc regardless of the ResourceVersion based on the above comment.

We should invoke syncScaledObject only if the spec has changed - do you see any other reason?

ahmelsayed · 2019-02-20T23:00:54Z

pkg/controller/controller.go


-	log.Infof("Starting autoscalers for: %s. target deployment: %s", scaledObject.GetName(), deployment.GetName())
+	go c.scaleHandler.HandleScale(scaledObject)


there is no need to block the controller on the ScaleHandler logic. This can happen async for every ScaledObject

I think we should care about order of operations, no? this is why the lock is in place. the goroutine defeats that purpose.

pkg/scalers/azure-queue-scaler.go

ahmelsayed · 2019-02-20T23:03:24Z

pkg/scalers/scale-handler.go

+		// scaledObjectCopy.Status.LastScaleTime = &currentTime
+		// scaledObjectCopy.Status.LastActiveTime = &currentTime
+		// scaledObjectCopy.Status.CurrentReplicas = *deploymentCopy.Spec.Replicas
+		// scaledObjectCopy.Status.DesiredReplicas = scaleDecision


For whatever reason, I couldn't get the Status of a ScaledObject to update. Everytime I'd call Update the next time it'll still be empty. Not sure why yet

better to replace the if-else of getScaler with switch sooner rather than after :)

About the status object not updating - can you try updating the state of the original object prior to the copy and see if it persists?

pkg/scalers/scale-handler.go

scale-handler.go: - Changed scaler ID to "solace-event-queue" solace_scaler.go: - changed metadata identifer prefixes from "msg" to "message" where applicable" - added multiplier to messageSpoolUsageTarget (1024^2) so metadata values are expressed in MBytes - updated some var names (camel case not followed after re-name) solace_scaler_test.go - Mods to reflect change in scaler code solace-helpers.ts - Mod to test procedure to allow message size to be specified for test publisher (test message spool usage) - Mod to trigger/scaler name in scaledobject config solace.tests.ts - added tests to scale up/down based on message spool usage (kedacore#5 and kedacore#6) (a) The contribution was created in whole or in part by me and I have the right to submit it under the open source license indicated in the file; or (b) The contribution is based upon previous work that, to the best of my knowledge, is covered under an appropriate open source license and I have the right under that license to submit that work with modifications, whether created in whole or in part by me, under the same open source license (unless I am permitted to submit under a different license), as indicated in the file; or (c) The contribution was provided directly to me by some other person who certified (a), (b) or (c) and I have not modified it. (d) I understand and agree that this project and the contribution are public and that a record of the contribution (including all personal information I submit with it, including my sign-off) is maintained indefinitely and may be redistributed consistent with this project or the open source license(s) involved. Signed-off-by: Dennis Brinley <dennis.brinley@solace.com>

Merge https://github.com/kedacore/keda:main into main

ahmelsayed added 11 commits February 19, 2019 13:56

Pin k8s to kubernetes-1.13.3 and add code-generator

a0fc28a

Add gengo and update generate-groups.sh

5e78326

Add initial iteration of scalers and Azure Queue Scaler

1af5bea

Remove secretRefs from ScaleTriggers OM

c16ba75

Add CircleCI build files

9400db6

Updating vendor packages

013b15a

Update CI build

fd95193

Add CI badge to README.md

b4f4f29

Add entrypoint to Dockerfile

194f3be

Add ScaleHandler and expand env vars in CONFIG

6eb74f6

Update README.md instructions

9244169

ahmelsayed requested review from anirudhgarg, Aarthisk and yaron2 February 20, 2019 22:49

ahmelsayed commented Feb 20, 2019

View reviewed changes

pkg/scalers/azure-queue-scaler.go Outdated Show resolved Hide resolved

ahmelsayed commented Feb 20, 2019

View reviewed changes

pkg/scalers/scale-handler.go Outdated Show resolved Hide resolved

ahmelsayed commented Feb 20, 2019

View reviewed changes

pkg/scalers/scale-handler.go Outdated Show resolved Hide resolved

ahmelsayed commented Feb 20, 2019

View reviewed changes

pkg/scalers/scale-handler.go Outdated Show resolved Hide resolved

ahmelsayed added 3 commits February 20, 2019 17:12

Update README.md

16b010f

Create scalers/azure_queue.go

435f5d0

Remove calls to DeepCopy

6b35e20

ahmelsayed merged commit fc7c8ef into master Feb 28, 2019

ahmelsayed mentioned this pull request Feb 28, 2019

Set resyncPeriod to 0 and add internal cache for scaledObjects #7

Merged

joelsmith pushed a commit to joelsmith/keda that referenced this pull request May 11, 2022

Merge pull request kedacore#6 from joelsmith/rebase-bot-main

dbe676d

Merge https://github.com/kedacore/keda:main into main

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Few updates #6

Few updates #6

ahmelsayed commented Feb 20, 2019 •

edited

Loading

ahmelsayed Feb 20, 2019

ahmelsayed Feb 20, 2019

yaron2 Feb 26, 2019

ahmelsayed Feb 20, 2019

yaron2 Feb 21, 2019

ahmelsayed Feb 25, 2019

yaron2 Feb 26, 2019

ahmelsayed Feb 20, 2019

yaron2 Feb 26, 2019

ahmelsayed Feb 20, 2019

yaron2 Feb 21, 2019

ahmelsayed Feb 20, 2019

yaron2 Feb 21, 2019

ahmelsayed Feb 25, 2019

yaron2 Feb 26, 2019


		log.Infof("Starting autoscalers for: %s. target deployment: %s", scaledObject.GetName(), deployment.GetName())
		go c.scaleHandler.HandleScale(scaledObject)

Few updates #6

Few updates #6

Conversation

ahmelsayed commented Feb 20, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ahmelsayed commented Feb 20, 2019 •

edited

Loading