Added resource limit worker for IIS webserver #33
Conversation
Signed-off-by: iamabhishek-dubey <[email protected]>
|
Ping @Vulfox |
|
CLA Signature Action: Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you read and sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just by adding a comment to this pull request with this exact sentence:
By commenting with the above message you are agreeing to the terms of the CLA. Your account will be recorded as agreeing to our CLA so you don't need to sign it again for future contributions to our company's repositories. 0 out of 1 committers have signed the CLA. |
Vulfox
left a comment
Vulfox
left a comment
There was a problem hiding this comment.
I was on a decent holiday break and I apologize for not seeing this sooner. I wanted to mute nearly everything :) .
Awesome stuff btw! I have a request to use nomad's stanza over creating our own, but other than that, this all looks top notch.
If you have a good counter point to anything I have mentioned, feel free to state them! Feedback is always welcome.
| "resource_limit": hclspec.NewBlock("resource_limit", false, hclspec.NewObject(map[string]*hclspec.Spec{ | ||
| "cpu_limit": hclspec.NewAttr("cpu_limit", "number", true), | ||
| "memory_limit": hclspec.NewAttr("memory_limit", "number", true), | ||
| })), |
There was a problem hiding this comment.
For this feature, I would prefer we use the resources stanza from Nomad and apply said values to IIS rather than configuring it within the driver config stanza.
https://www.nomadproject.io/docs/job-specification/resources
The resources stanza is a required setting for a nomad job spec, so we shouldn't have to worry about the value not being set. There might be a bit of conversion needed between nomad's value and types to what IIS is expecting, but this will facilitate a better UX for users who use other drivers and their settings.
| properties = []string{"set", "config", "-section:system.applicationHost/applicationPools"} | ||
|
|
||
| properties = append(properties, fmt.Sprintf("/[name='%s'].cpu.action:%s", appPoolName, "KillW3wp")) | ||
| properties = append(properties, fmt.Sprintf("/commit:apphost")) | ||
| if _, err := executeAppCmd(properties...); err != nil { | ||
| return fmt.Errorf("Failed to set Application Pool Resources: %v", err) | ||
| } |
There was a problem hiding this comment.
I would prefer this be a part of the CPULimit scope above as this pertains to configuring the CPU. Does applying this settings cause a noop if the CPULimit is left at 0?
|
Hey @Vulfox , |
|
@iamabhishek-dubey no worries! I had a decent break myself in Dec. Hope all is going well and looking forward to your commits. |
Signed-off-by: iamabhishek-dubey <[email protected]>
Signed-off-by: iamabhishek-dubey <[email protected]>
Signed-off-by: iamabhishek-dubey <[email protected]>
3 participants
Signed-off-by: iamabhishek-dubey [email protected]