Output to stderr when no log file path is passed

[couralex6]

What type of PR is this?
Bug fix

Which issue does this PR fix:
#1265

What does this PR do / Why do we need it:
Customers using Cilium in chaining mode were unable to create new pods after upgrading to CNI v1.7.x. Because no log file path was passed, zaplogger.go would default to stdout.
This change ensures that logs are being written to stderr when no log file path is passed.

If an issue # is not available please add repro steps and logs from IPAMD/CNI showing the issue:

Testing done on this change:
This was tested on a cluster with Cilium enabled in chaining mode . I reproduced the bug by installing CNI v1.7.5, which prevented new pods from being created. Pods were able to come up with this fix.

Automation added to e2e:
Added Unit Tests to test the following scenarios:

• Empty log file path > log to zapcore.Lock(os.Stderr)
• Actual file path > log to file path
• String set to 'stdout' > log to zapcore.Lock(os.Stdout)

Will this break upgrades or downgrades. Has updating a running cluster been tested?:
No

Does this change require updates to the CNI daemonset config files to work?:
No

Does this PR introduce any user-facing change?:
No

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[nithu0115]

Thanks for the fix @couralex6

[nithu0115]

@couralex6, could you please update documentation under AWS_VPC_K8S_PLUGIN_LOG_FILE env var, saying that if external plugins like Cilium and chained together, the conf file does not include pluginLogfile variable to pass in via stdin when CNI plugin is called by kubelet. This will result in no log files under /var/log/aws-routed-eni/plugin.log. However, result of CNI plugin execution can be seen in Kubelet logs as CNI plugin by default write output of cmdAdd to os.Stdout. (something along these lines for justification)

[jayanthvn]

@couralex6, could you please update documentation under AWS_VPC_K8S_PLUGIN_LOG_FILE env var, saying that if external plugins like Cilium and chained together, the conf file does not include pluginLogfile variable to pass in via stdin when CNI plugin is called by kubelet. This will result in no log files under /var/log/aws-routed-eni/plugin.log. However, result of CNI plugin execution can be seen in Kubelet logs as CNI plugin by default write output of cmdAdd to os.Stdout. (something along these lines for justification)

--> default to os.Stderr

[nithu0115]

Nice, @couralex6 /lgtm.

[nithu0115]

👍

[jayanthvn]

LGTM :)

[jayanthvn]

@couralex6 - This looks to be a private repo. Can you please check if we can use it?

[shaikatz]

Hi, when this PR is going to be released?

[jayanthvn]

Hi @shaikatz, We are planning for the next release, I will provide you the dates in a week or so.

[AmitBenAmi]

Hi @jayanthvn
There have been already 2 releases since 1.7.6, but none of them fixed this issue.
Is this fix expected to be on v1.7.9 or any other release expected to be released soon?

[jayanthvn]

Hi @AmitBenAmi

Sorry for the delay. Yes we are planning for a release In Jan/Feb. This will include this PR. Will update the exact date asap.

Thanks!

[yaron-idan]

Hey @jayanthvn, is there any update on the release date for this PR? We are still blocked from upgrading our CNI.

[fawadkhaliq]

@yaron-idan the fix will be included in the next patch release (1.7.9) and sorry, we don't share release dates publicly. Expect the patch release to be out in the coming days (very soon)