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

feat: add warning dialog message that virtual machine has low memory limit #2822

Merged
merged 1 commit into from
Jun 27, 2023

Conversation

vzhukovs
Copy link
Contributor

@vzhukovs vzhukovs commented Jun 12, 2023

What does this PR do?

The following changes request provides an additional confirmation dialog during kind creation request, if the particular container provider has lower than 6GB memory limit.

Screenshot/screencast of this PR

Monosnap Podman Desktop 2023-06-22 16-15-03

What issues does this PR fix or reference?

Fixes #2145

How to test this PR?

Configure Podman or Docker with less than 6GB ram and try to create a Kind Cluster. You should be warned with the message that it could be better to have a configured virtual machine with more than 6GB ram.

@vzhukovs vzhukovs requested review from a team and benoitf as code owners June 12, 2023 19:25
@vzhukovs vzhukovs requested review from jeffmaury and cdrage and removed request for a team June 12, 2023 19:25
@vzhukovs vzhukovs self-assigned this Jun 12, 2023
@deboer-tim
Copy link
Collaborator

Is it possible to embed this as a warning like PR #2671 instead of waiting until the user clicks Create?

I think this text would be a little clearer: "It is recommend to install Kind on a virtual machine with at least 6GB memory"

@vzhukovs
Copy link
Contributor Author

Is it possible to embed this as a warning like PR #2671 instead of waiting until the user clicks Create?

I think this text would be a little clearer: "It is recommend to install Kind on a virtual machine with at least 6GB memory"

Will try to do the same here

Comment on lines 253 to 254
// eslint-disable-next-line @typescript-eslint/no-explicit-any
consilienceCheck?(params: { [key: string]: any }): Promise<string[] | undefined>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hello, I would find another way. I have no clue if I implement something what would be a consilienceCheck

probably it should be an optional parameter of setContainerProviderConnectionFactory method. (or setKubernetesConnectionFactory method)

@vzhukovs
Copy link
Contributor Author

@deboer-tim like on the following mockup?
Monosnap Podman Desktop 2023-06-13 16-33-58

@deboer-tim
Copy link
Collaborator

Thank you, that banner is much better and more usable! The title 'Possible runtime error' is inconsistent with the icon though (is it an error or warning?), maybe something like 'Possible performance issue' or 'Virtual machine may be too small'.

@cdrage
Copy link
Contributor

cdrage commented Jun 15, 2023

Tested and the prompt came up / I was able to continue and create it. Just minor edits and I'm unsure with regards to the check function? But otherwise the code LGTM and the prompt is better than the modal

@jeffmaury jeffmaury added kind/feature 💡 Issue for requesting a new feature and removed kind/bug 🐞 Something isn't working labels Jun 16, 2023
@vzhukovs vzhukovs force-pushed the pd#2145 branch 2 times, most recently from 8f2f5db to faf2322 Compare June 23, 2023 12:52
@cdrage
Copy link
Contributor

cdrage commented Jun 26, 2023

Lint tests are failing, but otherwise LGTM on the new changes.

Signed-off-by: Vladyslav Zhukovskyi <vzhukovs@redhat.com>
@vzhukovs vzhukovs merged commit 4f38bd9 into main Jun 27, 2023
8 checks passed
@vzhukovs vzhukovs deleted the pd#2145 branch June 27, 2023 10:54
@podman-desktop-bot podman-desktop-bot added this to the 1.2.0 milestone Jun 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/extension/kind 🍾 kind/feature 💡 Issue for requesting a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Kind: Cluster running out of memory, Podman Machine default is 2GB. Increase to 6GB.
6 participants