fix: high resources panic #3984
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Javier López Barba <javier@okteto.com>
Signed-off-by: Javier López Barba <javier@okteto.com>
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #3984 +/- ##
==========================================
+ Coverage 41.93% 41.98% +0.04%
==========================================
Files 250 250
Lines 25015 25024 +9
==========================================
+ Hits 10490 10506 +16
+ Misses 13521 13516 -5
+ Partials 1004 1002 -2 |
pkg/k8s/deployments/crud.go
Outdated
| if limitCpu, ok := dev.Resources.Limits[apiv1.ResourceCPU]; ok { | ||
| manifestCpu = limitCpu.String() | ||
| maximumCpuPerPodMatchGroups := cpuMaximumRegex.FindStringSubmatch(errorMessage) | ||
| if len(maximumCpuPerPodMatchGroups) < 2 { |
There was a problem hiding this comment.
Why 2, can we put this value under a const with name?
pkg/k8s/deployments/crud.go
Outdated
| if limitMemory, ok := dev.Resources.Limits[apiv1.ResourceMemory]; ok { | ||
| manifestMemory = limitMemory.String() | ||
| maximumMemoryPerPodMatchGroups := memoryMaximumRegex.FindStringSubmatch(errorMessage) | ||
| if len(maximumMemoryPerPodMatchGroups) < 2 { |
teresaromero
approved these changes
Sep 21, 2023
AdrianPedriza
approved these changes
Sep 21, 2023
pkg/k8s/deployments/crud_test.go
Outdated
| if err.Error() != tt.expectedErr.Error() { | ||
| t.Fatalf("wrong error. Got %s, expected %s", err, tt.expectedErr) | ||
| if err != nil && tt.expectedErr == nil { | ||
| t.Fatalf("wrong error. Got nil, expected %s", err.Error()) |
There was a problem hiding this comment.
Suggested change
| t.Fatalf("wrong error. Got nil, expected %s", err.Error()) | |
| t.Fatalf("wrong error. Expected nil, but got %s", err.Error()) |
andreafalzetti
approved these changes
Sep 25, 2023
Signed-off-by: Javier López Barba <javier@okteto.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed changes
Fixes #3841
Fix a panic that was happening when the message produced by k8s was missing any resource and limit. Given that I was not able to reproduce it, I added tests covering the use cases where the error could happen based on the logs provided.
How to validate
Check unit tests. No way to reproduce the issue on any cluster. I just could reproduce it on unit tests.
CLI Quality Reminders 🔧
For both authors and reviewers: