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 eck-operator memory requests/limits to LogStorage CR. #1137

Conversation

vberezny
Copy link
Contributor

@vberezny vberezny commented Feb 3, 2021

Description

[Bug Fix] Exposes the elastic-operator pod memory requests/limits in the LogStorage CR. This was done to allow the end user to easily get around issues with elastic-operator pod being OOM killed due to memory usage.

Tested on the local kind cluster as well as in a banzai provisioned gcp-kubeadm environment.

https://tigera.atlassian.net/browse/SAAS-1250

Corresponding Docs PR:
https://github.com/tigera/calico-private/pull/2986

For PR author

  • [x ] Tests for change.
  • [x ] If changing pkg/apis/, run make gen-files

For PR reviewers

A note for code reviewers - all pull requests must have the following:

  • Milestone set according to targeted release.
  • Appropriate labels:
    • kind/bug if this is a bugfix.
    • kind/enhancement if this is a a new feature.
    • enterprise if this PR applies to Calico Enterprise only.

@CLAassistant
Copy link

CLAassistant commented Feb 3, 2021

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@tmjd tmjd left a comment

Choose a reason for hiding this comment

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

Looking good, I think a few additions are needed though.
In addition to some validation (see my comments) I'd also like to see UTs for the validation.
Also make sure you run make gen-files to update the CRDs and deepcopy, I pulled your branch and ran it and saw a CRD update.

api/v1/installation_types.go Show resolved Hide resolved
@vberezny vberezny force-pushed the vlad-SAAS-1250-expose-memory-settings-eck-operator branch from 14bd363 to e980221 Compare February 5, 2021 00:06
@vberezny vberezny requested a review from tmjd February 5, 2021 00:10
pkg/controller/installation/core_controller.go Outdated Show resolved Hide resolved
pkg/controller/installation/core_controller.go Outdated Show resolved Hide resolved
pkg/controller/installation/core_controller.go Outdated Show resolved Hide resolved
pkg/controller/installation/core_controller_test.go Outdated Show resolved Hide resolved
pkg/controller/logstorage/logstorage_controller.go Outdated Show resolved Hide resolved
pkg/controller/logstorage/logstorage_controller.go Outdated Show resolved Hide resolved
pkg/render/logstorage_test.go Outdated Show resolved Hide resolved
@vberezny vberezny force-pushed the vlad-SAAS-1250-expose-memory-settings-eck-operator branch from e980221 to a580b41 Compare February 8, 2021 21:28
@vberezny vberezny requested a review from tmjd February 8, 2021 21:30
@vberezny vberezny force-pushed the vlad-SAAS-1250-expose-memory-settings-eck-operator branch 5 times, most recently from e7d884e to e9e5888 Compare February 8, 2021 22:58
@vberezny vberezny force-pushed the vlad-SAAS-1250-expose-memory-settings-eck-operator branch from e9e5888 to 3b40664 Compare February 8, 2021 23:20
Copy link
Member

@tmjd tmjd left a comment

Choose a reason for hiding this comment

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

LGTM

@tmjd tmjd merged commit 2a9df8d into tigera:master Feb 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants