-
Notifications
You must be signed in to change notification settings - Fork 614
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
Silence WARN[0000] vmType vz: ignoring networks[0]: [Metric]
#3010
Conversation
Fix lima-vm#2986 Signed-off-by: Jan Dubois <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
@@ -130,6 +130,7 @@ func (l *LimaVzDriver) Validate() error { | |||
"Lima", | |||
"Socket", | |||
"MACAddress", | |||
"Metric", | |||
"Interface", | |||
); len(unknown) > 0 { | |||
logrus.Warnf("vmType %s: ignoring networks[%d]: %+v", *l.Instance.Config.VMType, i, unknown) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is easy to grep for the warning text and understand why it happens but the warning should make it clear that this is a validation warning, and the issue is unknown property. Maybe something like:
validate vmType vz: ignoring unknown property networks[1]: "Metric"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SGTM
If you are going to work on this, you should also take a look at the corresponding code in the pkg/wsl2/wsl_driver_windows.go
file to try to keep things in sync (I noticed vz uses ignoring
while wsl2
uses Ignoring
, which caused me to miss them on my initial grepping).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course validation is going to be a big rabbit hole; look at #2512 for more thoughts on how it should be refactored.
Personally I don't want to touch it (validation) until I had a chance to replace the current FillDefaults
with the upcoming code from template assembly because that might change things around anyways.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have time for this now, added #3011
Fix #2986