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

Add docs for external-push scaler #193

Merged
merged 11 commits into from
Jul 29, 2020
Merged

Conversation

ahmelsayed
Copy link
Contributor

Signed-off-by: Ahmed ElSayed ahmels@microsoft.com

Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@tomkerkhove tomkerkhove left a comment

Choose a reason for hiding this comment

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

Can we provide a link or some guidance on how to build one of these?

@ahmelsayed
Copy link
Contributor Author

Yes, I'll put something together for that

@zroubalik
Copy link
Member

@ahmelsayed any updated on the guidance doc pls?

@tomkerkhove
Copy link
Member

Bump @ahmelsayed

@ahmelsayed
Copy link
Contributor Author

Sorry about the delay, I'll get it done this week.

Signed-off-by: Ahmed ElSayed <ahmels@microsoft.com>
@ahmelsayed
Copy link
Contributor Author

@tomkerkhove I added this guide. I tried to add a tabs shortcode like this one but using aplinejs instead of jquery, but after few hours I couldn't get it to work, so I just repurposed the component for FAQ. If someone knows alpinejs and hugo better than me, it would be nice to have the different languages in tabs rather than collapsible components, I think

https://deploy-preview-193--keda.netlify.app/docs/2.0/concepts/external-scalers/

Signed-off-by: Ahmed ElSayed <ahmels@microsoft.com>
Copy link
Member

@tomkerkhove tomkerkhove left a comment

Choose a reason for hiding this comment

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

LGTM, added a few suggestions.

Can we do the same for v1.5 & v1.4 or is it too much effort?

content/docs/2.0/concepts/external-scalers.md Show resolved Hide resolved
content/docs/2.0/concepts/external-scalers.md Show resolved Hide resolved
content/docs/2.0/concepts/external-scalers.md Show resolved Hide resolved
content/docs/2.0/concepts/external-scalers.md Outdated Show resolved Hide resolved

{{< collapsible "Golang" >}}

Full implementation can be found here: https://github.com/ahmelsayed/external-scaler-samples
Copy link
Member

Choose a reason for hiding this comment

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

Can we donate this to kedacore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, was just preparing the repo. Will move it there.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks and also for the repo, very useful to have a reference as well!

content/docs/2.0/concepts/external-scalers.md Outdated Show resolved Hide resolved
```

The `Scaler` interface defines 4 methods:
- `GetMetrics` and `GetMetricsSpecForScaling` that are used to build the HPA definition for that particular scaler.
Copy link
Member

Choose a reason for hiding this comment

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

What should we expect here? It's not fully clear to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a bit more details about them in the "Implementing" sections

GetMetricSpec returns the target value for the HPA definition for the scaler. This scaler will define a static target of 10, but the threshold value is often specified in the metadata for other scalers.

https://github.com/ahmelsayed/keda-docs/blob/push-scaler/content/docs/2.0/concepts/external-scalers.md#6-implementing-getmetrics

GetMetrics returns the value of the metric refered to from GetMetricSpec, in this example it's earthquakeThreshold. Refer to the HPA docs for how HPA calculates replicaCount based on metric value and target value.

https://github.com/ahmelsayed/keda-docs/blob/push-scaler/content/docs/2.0/concepts/external-scalers.md#6-implementing-getmetrics

It's easier to explain with an example, but I can expand the description there a bit. how about

GetMetricSpec returns the target value for the HPA definition for the scaler. For more details refer to Implementing GetMetricSpec

GetMetrics returns the value of the metric refered to from GetMetricSpec. For more details refer to Implementing GetMetrics

?

Copy link
Member

Choose a reason for hiding this comment

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

It helps a lot, thanks!

Examples are always welcome but not a must!

content/docs/2.0/concepts/external-scalers.md Outdated Show resolved Hide resolved
content/docs/2.0/concepts/external-scalers.md Outdated Show resolved Hide resolved
scalerAddress: external-scaler-service:8080
tlsCertFile: /path/to/tls/cert.pem # optional
```

Copy link
Member

Choose a reason for hiding this comment

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

I would add a link here maybe on how to write an external push scaler (concepts)?

Same goes for the external scaler.

ahmelsayed and others added 6 commits July 28, 2020 11:07
Co-authored-by: Tom Kerkhove <kerkhove.tom@gmail.com>
Co-authored-by: Tom Kerkhove <kerkhove.tom@gmail.com>
Co-authored-by: Tom Kerkhove <kerkhove.tom@gmail.com>
Co-authored-by: Tom Kerkhove <kerkhove.tom@gmail.com>
Co-authored-by: Tom Kerkhove <kerkhove.tom@gmail.com>
Signed-off-by: Ahmed ElSayed <ahmels@microsoft.com>
Signed-off-by: Ahmed ElSayed <ahmels@microsoft.com>
Copy link
Member

@tomkerkhove tomkerkhove left a comment

Choose a reason for hiding this comment

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

Other than the current comments it looks good to me, thanks!

content/docs/2.0/concepts/external-scalers.md Outdated Show resolved Hide resolved
ahmelsayed and others added 2 commits July 29, 2020 16:07
Co-authored-by: Tom Kerkhove <kerkhove.tom@gmail.com>
Signed-off-by: Ahmed ElSayed <ahmels@microsoft.com>
@ahmelsayed ahmelsayed merged commit 658f24b into kedacore:master Jul 29, 2020
@ahmelsayed ahmelsayed deleted the push-scaler branch July 29, 2020 23:20
@ahmelsayed
Copy link
Contributor Author

I'll open other PRs for porting the guide to 1.5 and 1.4

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.

3 participants