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

Added support for resources request in staging jobs #2384

Merged
merged 2 commits into from
Jun 19, 2023

Conversation

andreas-kupries
Copy link
Contributor

Fix #2286
Ref epinio/helm-charts#439

feat: staging job resource request support

@andreas-kupries
Copy link
Contributor Author

Testing that the new facility operates as desired:

  • Install E with --set server.stagingResourceRequests.cpu=100m --set server.stagingResourceRequests.memory=100M
  • Use kubectl get pod -o yaml .... to look at the YAML of the epinio-server pod and check that the STAGING_RESOURCE_... environment variables are set to the requested values.
  • Push an application and inspect the stage job pod to also have these values

Example server data

    - name: STAGING_RESOURCE_CPU
      value: 100m
    - name: STAGING_RESOURCE_MEMORY
      value: 100M

Example job data

spec:
  containers:
    - [...]
      resources:
        requests:
          cpu: 100m
          memory: 100M

@andreas-kupries andreas-kupries force-pushed the 2286-staging-job-resource-support branch from 5ffddf8 to 89f196b Compare June 16, 2023 12:29
@codecov
Copy link

codecov bot commented Jun 16, 2023

Codecov Report

Patch coverage: 74.81% and project coverage change: +0.39 🎉

Comparison is base (fbd6f8f) 65.33% compared to head (5ffddf8) 65.72%.

❗ Current head 5ffddf8 differs from pull request most recent head 89f196b. Consider uploading reports for the commit 89f196b to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2384      +/-   ##
==========================================
+ Coverage   65.33%   65.72%   +0.39%     
==========================================
  Files         181      181              
  Lines       15412    15514     +102     
==========================================
+ Hits        10069    10197     +128     
+ Misses       4188     4168      -20     
+ Partials     1155     1149       -6     
Flag Coverage Δ
acceptance-api 47.73% <45.18%> (+<0.01%) ⬆️
acceptance-apps 38.16% <26.66%> (-0.08%) ⬇️
acceptance-cli 62.22% <74.07%> (+0.34%) ⬆️
unittests 9.72% <11.76%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
internal/api/v1/configuration/create.go 72.22% <ø> (-2.14%) ⬇️
internal/configurations/configurations.go 80.83% <0.00%> (-1.03%) ⬇️
internal/api/v1/application/deploy.go 56.66% <40.00%> (-2.27%) ⬇️
internal/api/v1/application/stage.go 75.78% <44.44%> (-0.49%) ⬇️
internal/cli/server.go 85.00% <61.53%> (-3.97%) ⬇️
internal/cli/configuration.go 66.82% <92.30%> (+8.39%) ⬆️
internal/api/v1/application/restart.go 63.26% <100.00%> (-0.74%) ⬇️
internal/api/v1/application/update.go 66.32% <100.00%> (ø)
internal/api/v1/configuration/replace.go 59.61% <100.00%> (-0.77%) ⬇️
internal/api/v1/configuration/update.go 58.00% <100.00%> (-0.83%) ⬇️
... and 6 more

... and 7 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@andreas-kupries andreas-kupries marked this pull request as ready for review June 16, 2023 13:17
@andreas-kupries andreas-kupries requested a review from a team as a code owner June 16, 2023 13:17
Copy link
Contributor

@thehejik thehejik left a comment

Choose a reason for hiding this comment

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

Works as expected.

Versions
Kubernetes Version: v1.26.5+k3s1
Epinio Server Version: v1.9.0-rc2-18-g89f196b1

Validation

 --set server.stagingResourceRequests.cpu=100m --set server.stagingResourceRequests.memory=100M
  • Epinio-server deployment patched to use epinio binary built from this PR, used make patch-epinio-deployment
  • Checked environment of epinio-server deployment:
$ kubect get deploy  -n epinio epinio-server -o jsonpath='{..env}' | jq
...
  {
    "name": "STAGING_RESOURCE_CPU",
    "value": "100m"
  },
  {
    "name": "STAGING_RESOURCE_MEMORY",
    "value": "100M"
  }
...
  • Pushed an app and checked its staging job:
$ kubectl get pod stage-workspace-sample-70bb266339423ee24e42c4426abd2f15cddrrlzs -n epinio -o yaml
...
resources:
  requests:
    cpu: 100m
    memory: 100M
...
  • The deployed app itself doesn't have the resources set: resources: {}
  • Input values validation check:
    • With --set server.stagingResourceRequests.cpu=100X - the helm installation fails on CrashLoopBackOff for epinio-server pod, log contains:
    ❌  bad cpu request for staging job: quantities must match the regular expression '^([+-]?[0-9.]+)([eEinumkKMGTP]*[-+]?[0-9]*)$'
    
    • With --set server.stagingResourceRequests.memory=a100hy - the helm installation fails on CrashLoopBackOff for epinio-server pod, log contains:
    ❌  bad memory request for staging job: quantities must match the regular expression '^([+-]?[0-9.]+)([eEinumkKMGTP]*[-+]?[0-9]*)$'
    
  • User can set both resource types at once or just one of them

@andreas-kupries
Copy link
Contributor Author

It should be possible to set both cpu and memory.

@andreas-kupries andreas-kupries merged commit 957319b into main Jun 19, 2023
17 checks passed
@andreas-kupries andreas-kupries deleted the 2286-staging-job-resource-support branch June 19, 2023 13:49
@thehejik
Copy link
Contributor

thehejik commented Jul 4, 2023

Verified in epinio v1.9.0-rc3 according to the report above - all good

@enrichman enrichman changed the title Support resource requests for staging jobs Added support for resource requests in staging jobs Jul 20, 2023
@enrichman enrichman changed the title Added support for resource requests in staging jobs Added support for resources request in staging jobs Jul 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Resources Requests for Staging Job
2 participants