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

[Merged by Bors] - Set explicit resources on all containers #345

Closed
wants to merge 26 commits into from

Conversation

razvan
Copy link
Member

@razvan razvan commented Jun 6, 2023

Raise postgresql chart version.

Description

Part of: stackabletech/issues#394

Definition of Done Checklist

  • Not all of these items are applicable to all PRs, the author should update this template to only leave the boxes in that are relevant
  • Please make sure all these things are done and tick the boxes

Author

Preview Give feedback

Reviewer

Preview Give feedback

Acceptance

Preview Give feedback

Once the review is done, comment bors r+ (or bors merge) to merge. Further information

Raise postgresql chart version.
rust/operator-binary/src/controller.rs Outdated Show resolved Hide resolved
rust/operator-binary/src/controller.rs Outdated Show resolved Hide resolved
rust/operator-binary/src/controller.rs Outdated Show resolved Hide resolved
rust/operator-binary/src/controller.rs Outdated Show resolved Hide resolved
@razvan razvan marked this pull request as ready for review June 16, 2023 15:04
@razvan razvan requested a review from a team June 16, 2023 15:04
@sbernauer
Copy link
Member

sbernauer commented Jun 19, 2023

  • Change resource defaults
  • Update docs with
    • default resources
    • What a minimal HA setup needs
  • (optional) implement podOverrrides
    • docs
      • Link to concepts page
      • Document list of all configuration files that you can configure
      • Document list of (init + app) containers that users can configure using podOverrrides
      • Document if there are any things (e.g. superst init-db-job) that you can't influence using podOverrides
      • Document if there are any env variables that you can't influence using envOverrides, e.g. because k8s set's them or env var is set from entrypoint script (e.g. KERBEROS_REAL from /stackable/kerberos/krb5.conf)

@sbernauer sbernauer self-requested a review June 19, 2023 12:54
@sbernauer
Copy link
Member

I would rename the PR to Set explicit resources on all container

@razvan razvan changed the title Smoke tests with resource quotas Set explicit resources on all container Jun 19, 2023
@razvan razvan changed the title Set explicit resources on all container Set explicit resources on all containers Jun 19, 2023
CHANGELOG.md Outdated Show resolved Hide resolved
Cargo.toml Show resolved Hide resolved
rust/crd/src/lib.rs Outdated Show resolved Hide resolved
docs/modules/hive/pages/usage.adoc Outdated Show resolved Hide resolved
docs/modules/hive/pages/usage.adoc Outdated Show resolved Hide resolved
docs/modules/hive/pages/usage.adoc Outdated Show resolved Hide resolved
tests/templates/kuttl/smoke/00-limit-range.yaml Outdated Show resolved Hide resolved
tests/templates/kuttl/smoke/60-install-hive.yaml.j2 Outdated Show resolved Hide resolved
razvan and others added 3 commits June 19, 2023 15:29
Co-authored-by: Sebastian Bernauer <sebastian.bernauer@stackable.de>
@razvan razvan requested a review from sbernauer June 19, 2023 13:51
Copy link
Member

@sbernauer sbernauer left a comment

Choose a reason for hiding this comment

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

Thx!

tests/templates/kuttl/smoke/00-limit-range.yaml Outdated Show resolved Hide resolved
@razvan razvan requested a review from sbernauer June 19, 2023 14:19
Copy link
Member

@sbernauer sbernauer left a comment

Choose a reason for hiding this comment

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

Many thanks! LGTM when tests pass

docs/modules/hive/pages/usage.adoc Outdated Show resolved Hide resolved
docs/modules/hive/pages/usage.adoc Outdated Show resolved Hide resolved
rust/operator-binary/src/controller.rs Outdated Show resolved Hide resolved
tests/templates/kuttl/smoke/00-limit-range.yaml Outdated Show resolved Hide resolved
razvan and others added 4 commits June 19, 2023 16:25
Co-authored-by: Sebastian Bernauer <sebastian.bernauer@stackable.de>
Co-authored-by: Sebastian Bernauer <sebastian.bernauer@stackable.de>
Co-authored-by: Sebastian Bernauer <sebastian.bernauer@stackable.de>
@razvan
Copy link
Member Author

razvan commented Jun 20, 2023

All tests pass on AKS 1.26:

--- PASS: kuttl (484.15s)
    --- PASS: kuttl/harness (0.00s)
        --- PASS: kuttl/harness/logging_postgres-12.5.6_hive-2.3.9-stackable0.0.0-dev_openshift-false (83.63s)
        --- PASS: kuttl/harness/orphaned-resources_hive-latest-3.1.3-stackable0.0.0-dev (106.56s)
        --- PASS: kuttl/harness/resources_hive-3.1.3-stackable0.0.0-dev (32.01s)
        --- PASS: kuttl/harness/resources_hive-2.3.9-stackable0.0.0-dev (32.98s)
        --- PASS: kuttl/harness/cluster-operation_hive-latest-3.1.3-stackable0.0.0-dev (129.28s)
        --- PASS: kuttl/harness/smoke_postgres-12.5.6_hive-3.1.3-stackable0.0.0-dev_openshift-false_s3-use-tls-false (99.95s)
        --- PASS: kuttl/harness/smoke_postgres-12.5.6_hive-2.3.9-stackable0.0.0-dev_openshift-false_s3-use-tls-true (117.36s)
        --- PASS: kuttl/harness/smoke_postgres-12.5.6_hive-3.1.3-stackable0.0.0-dev_openshift-false_s3-use-tls-true (136.84s)
        --- PASS: kuttl/harness/logging_postgres-12.5.6_hive-3.1.3-stackable0.0.0-dev_openshift-false (83.30s)
        --- PASS: kuttl/harness/smoke_postgres-12.5.6_hive-2.3.9-stackable0.0.0-dev_openshift-false_s3-use-tls-false (117.46s)
PASS

@razvan
Copy link
Member Author

razvan commented Jun 20, 2023

bors merge

bors bot pushed a commit that referenced this pull request Jun 20, 2023
Raise postgresql chart version.


# Description

Part of: stackabletech/issues#394



Co-authored-by: Sebastian Bernauer <sebastian.bernauer@stackable.de>
@bors
Copy link
Contributor

bors bot commented Jun 20, 2023

Pull request successfully merged into main.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot changed the title Set explicit resources on all containers [Merged by Bors] - Set explicit resources on all containers Jun 20, 2023
@bors bors bot closed this Jun 20, 2023
@bors bors bot deleted the feat/resource-quotas branch June 20, 2023 11:24
@sbernauer
Copy link
Member

Two questions:

  1. Will the Statefulset request a pvc in any case (e.g. the default config?
  2. It seems a bit weird that vector get's the same resource as the hive container. We do also deviate from the pattern that "things that are breaking stuff when slow" have a ration of 2, this PR added a ration of 4. Was there any measurements to get to this cpu resources?

@razvan
Copy link
Member Author

razvan commented Jun 21, 2023

  1. No. There is never a PVC created.
  2. I thought the factor of 4 or 5 is a maximum limit not an fixed absolute value.

@sbernauer
Copy link
Member

  1. Awesome, thx!
  2. It's a maximum limit we set ourselves, the 2 is just an idea that I personally had to guarantee that important stuff will keep running, even when the k8s cluster is heavily loaded

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants