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

fix: don't define resources #297

Merged
merged 3 commits into from
Jul 6, 2023
Merged

Conversation

rissson
Copy link
Contributor

@rissson rissson commented May 25, 2023

what

Remove cpu limits from the atlantis container

why

Atlantis simply dies on simple tasks like terraform init with the current configuration.

tests

  • I have tested my changes by running them in production.

references

https://home.robusta.dev/blog/stop-using-cpu-limits

@rissson rissson requested a review from a team as a code owner May 25, 2023 20:28
@jseiser
Copy link
Contributor

jseiser commented May 26, 2023

We should not specify any resources.

This entire section should default to nothing.

resources:
  requests:
    memory: 1Gi
    cpu: 100m
  limits:
    memory: 1Gi
    # cpu: 100m

@rissson rissson changed the title fix: remove cpu limits fix: don't define resources May 26, 2023
nitrocode
nitrocode previously approved these changes May 26, 2023
@jseiser
Copy link
Contributor

jseiser commented May 30, 2023

@nitrocode can this be merged without bumping the chart version? Or will a github action handle that?

@GenPage
Copy link
Member

GenPage commented Jun 6, 2023

@jseiser The github action will not handle it. It will need a bump.

@GenPage
Copy link
Member

GenPage commented Jun 6, 2023

The lint check fixes now catches a missing version bump

Copy link
Member

@GMartinez-Sisti GMartinez-Sisti left a comment

Choose a reason for hiding this comment

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

LGTM, I just think this should bump patch instead of minor since it's not implementing a feature.

Signed-off-by: Marc 'risson' Schmitt <marc.schmitt@risson.space>
@rissson
Copy link
Contributor Author

rissson commented Jun 21, 2023

Updated as required.

@jseiser
Copy link
Contributor

jseiser commented Jul 6, 2023

@GMartinez-Sisti
@GenPage
@rissson

Any chance we can get the chart version updated again, and get this one merged?

Thanks,

@rissson
Copy link
Contributor Author

rissson commented Jul 6, 2023

Maintainers have write access to this branch.

@jseiser
Copy link
Contributor

jseiser commented Jul 6, 2023

We enabled parallel plans, and now we are hitting this same problem. a MR is created and atlantis dies.

@jamengual
Copy link
Contributor

@rissson can you please bump the version? usually, the user does this instead of us

@jamengual jamengual merged commit 710bba1 into runatlantis:main Jul 6, 2023
2 checks passed
@rissson rissson deleted the no-cpu-limits branch July 6, 2023 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants