Etcd memory and cpu requests

[integrii]

Some of our etcd pods end up with etcd memory use that far exceeds the request defaults kops applies. This change adds the ability to specify the pod memory and CPU requests on a per-etcd-cluster basis so that we can up our requests. It also adds a default 200m CPU to etcd clusters named main, and 100m otherwise. 100Mi of memory is always assigned by default. This should match prior functionality.

Two new options open up for etcd cluster configuration. cpuRequest and memoryRequest. These, when set, of course override the defaults described here.

There are plenty of things I can see that could be improved, like using a constant for the default memory and CPU. I just wasn't sure where to put it. It took me about two days to find all these parts of the code, work around bugs with things like api machinery. Feel free to let me know if something needs refactored or adjusted.

Thanks!

[k8s-ci-robot]

Hi @integrii. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[integrii]

@ihoegen 😻

[ihoegen]

/ok-to-test

[ihoegen]

/assign ihoegen

[ihoegen]

I think you're going to need to add this to

k8s.io/kops/pkg/apis/kops/v1alpha1/cluster.go
k8s.io/kops/pkg/apis/kops/v1alpha2/cluster.go

[ihoegen]

Be sure to run make apimachinery && make locally too, as well as adding the field to v1alpha1 and v1alpha2

[integrii]

I added the config lines to the other cluster config versions, then I hit #6314 when building api machinery. I unset my GOBIN to bypass the error for now.

Once that worked, I get this error:

E0108 22:15:49.365750   43694 conversion.go:667] Warning: could not find nor generate a final Conversion function for k8s.io/kops/pkg/apis/kops.EtcdClusterSpec -> k8s.io/kops/pkg/apis/kops/v1alpha1.EtcdClusterSpec
E0108 22:15:49.365777   43694 conversion.go:668]   the following fields need manual conversion:
E0108 22:15:49.365782   43694 conversion.go:670]       - MemoryRequest
E0108 22:15:49.365786   43694 conversion.go:670]       - CPURequest
/Users/egreer200/go/bin/conversion-gen  --skip-unsafe=true --input-dirs k8s.io/kops/pkg/apis/kops/v1alpha2 --v=0  --output-file-base=zz_generated.conversion \
		 --go-header-file "hack/boilerplate/boilerplate.go.txt"
E0108 22:15:51.688471   43695 conversion.go:667] Warning: could not find nor generate a final Conversion function for k8s.io/kops/pkg/apis/kops.EtcdClusterSpec -> k8s.io/kops/pkg/apis/kops/v1alpha2.EtcdClusterSpec
E0108 22:15:51.688496   43695 conversion.go:668]   the following fields need manual conversion:
E0108 22:15:51.688500   43695 conversion.go:670]       - MemoryRequest
E0108 22:15:51.688504   43695 conversion.go:670]       - CPURequest

How do I add said manual conversion?

[ihoegen]

See k8s.io/kops/pkg/apis/kops/v1alpha1/zz_generated.conversion.go, there is auto generated comments on those warnings

[integrii]

I added the stuff generated from the apimachinery run - was that right??

[integrii]

lol - did the files that it generates fail the go fmt test?

[ihoegen]

Yeah, looks like it's because of some whitespace. Try running make gofmt first

[integrii]

i just ran go fmt on everything and found nothing. make gofmt does not find any changes, either. I will take a closer look at the errors here tomorrow.

[integrii]

@ihoegen I gave you access to my branch. Can you run a format on it pretty please? I'm guessing there is a tooling difference that will be annoying to find...

[ihoegen]

Gotta fix the Travis CI tests now: https://travis-ci.org/kubernetes/kops/jobs/477460494

Bunch of stuff regarding expected cluster specs

[k8s-ci-robot]

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

---

• If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
• If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
• If you have done the above and are still having issues with the CLA being reported as unsigned, please email the CNCF helpdesk: [email protected]

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[integrii]

Whew, okay that was a bit gory. I had to back out a merge from master I did before I could rebase to squash commits because it wanted to rebase everything that had happened to master...

We should be down to just one commit from me and two from Ian, to maintain his contributions.

I am now going to re-merge master and re-resolve conflicts since the rebase is complete.

[integrii]

Finally I was able to squash all my contiguous commits between upstream merges. Whoo!

[KashifSaadat]

That looked painful! Thanks for sorting out all the conflicts and squashing the commits :) I've dropped you a message on Slack in regards to the test fix, specifically in the pkg/model/bootstrapscript.go file.

[integrii]

@KashifSaadat you're my hero. I made the change - lets see if builds pass!

[KashifSaadat]

/test pull-kops-e2e-kubernetes-aws

[KashifSaadat]

All green! Thanks @integrii 👍

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ihoegen, integrii, KashifSaadat

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [KashifSaadat]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[integrii]

Excellent!! I will try to make time to hit up the Calico specs next.