Mg/refactor summary #90
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
mgoodhart5
commented
Oct 31, 2019
mgoodhart5
commented
- refactored summary package with Client struct
- run func
- constructSummary func
- refactored TestRunSummary
Codecov Report
@@ Coverage Diff @@
## master #90 +/- ##
==========================================
+ Coverage 39.4% 42.77% +3.37%
==========================================
Files 8 8
Lines 538 547 +9
==========================================
+ Hits 212 234 +22
+ Misses 316 302 -14
- Partials 10 11 +1
Continue to review full report at Codecov.
|
dosullivan
approved these changes
Nov 1, 2019
|
@mgoodhart5 Would you mind rebasing and resolving the conflicts on this? |
sudermanjr
force-pushed
the
mg/refactor-summary
branch
from
87af1b1
to
2c137bf
Compare
mgoodhart5
force-pushed
the
mg/refactor-summary
branch
from
2c137bf
to
c119e2d
Compare
rbren
reviewed
Nov 5, 2019
sudermanjr
requested changes
Nov 12, 2019
sudermanjr
approved these changes
Nov 14, 2019
hjkatz
pushed a commit
to hjkatz/goldilocks
that referenced
this pull request
Apr 13, 2020
This reverts commit 998db15.
Merged
hjkatz
pushed a commit
to hjkatz/goldilocks
that referenced
this pull request
May 4, 2020
This reverts commit 998db15.
sudermanjr
pushed a commit
that referenced
this pull request
May 5, 2020
…154) * Revert "Make units consistent for memory fixes issue #78 (#103)" This reverts commit e55026d. * Add FormatResourceList and test cases * Use v1 resource memory constants instead of hardcoded strings * Add a VPAUpdateMode to the vpa.Reconciler The VPAUpdateMode field will allow us to use Goldilocks to create VPAs that operate outside of "off" mode. * Add ability to specify vpa update mode This commit adds a parameter to createVPA that allows the user to specify the update mode for the VPA. The mechanism for specifying this is the label goldilocks.fairwinds.com/vpaUpdateMode. * Extract variables instead of hardcoded values * VPA update mode label case insensitive Kubernetes labels are based on DNS names, which are case insensitive. However, the current label used to specify the update mode of created VPAs, vpaUpdateMode, is case sensitive. This has caused a bug that results in all VPAs being created in "Off" mode. This commit fixes that bug by using a case-insensitive label to specify the vpa-update-mode. * Summary fix for multiple containers in a pod (#123) * go.mod updated for some reason * Add support for all VPA UpdateModes * Use ParseBool for deployment enabled label value * Use ParseBool for namespace enabled label value * Use lowercase for switch statement * Add Deployment Exclusion Annotation Also: - Update reconciler to use VPA and Deployment objects directly (instead of names) - Update reconciler logging - Update/Add vpa tests * Change no-vpa annotation to vpa-opt-out (to opt out of vpa in auto mode) Also: - Update vpa tests - Fix vpa-opt-out logic * Add more logging to VPA reconciler * Fix namespace logs to use Name instead of the full Namespace * Update README * Fix Typo aut -> auto Co-Authored-By: Andrew Suderman <andrew@sudermanjr.com> * Fix grammar matched -> match Co-Authored-By: Andrew Suderman <andrew@sudermanjr.com> * Use vpa-update-mode as an annotation for Deployment specific VPA control * Fix ineffectual assignment * Fix test label * Use index to get correct vpa * Use short-hand if statement for map lookup * Expand the functionality and interface for pkg/summary * Add namespace support for summary command * Standardize Vpa -> VPA * Remove unused cached summary * Fix typo in summary command help * Remove confusing error comment * Revert "Refactored summary pkg and added more tests. (#90)" This reverts commit 998db15. * Remove superfluous v1 import alias * Use var for empty slice initialization Co-Authored-By: Luke Reed <luke@lreed.net> * Report an error when too many namespace arguments are given to the summary command * golint and fixes * Use apps/v1 for summary_test.go * Re-export Option type (accidentally unexported) * Use --namespace flag for summary command instead of args Co-authored-by: John Turner <jturner@squarespace.com> Co-authored-by: David Noyes <david.noyes@skybettingandgaming.com> Co-authored-by: Andrew Suderman <andrew@sudermanjr.com> Co-authored-by: Luke Reed <luke@lreed.net>
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.