Issue 8032: make node agent configMap name configurable #8097
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8097 +/- ##
==========================================
+ Coverage 58.77% 59.01% +0.23%
==========================================
Files 358 364 +6
Lines 30070 30270 +200
==========================================
+ Hits 17674 17864 +190
- Misses 10949 10960 +11
+ Partials 1447 1446 -1 ☔ View full report in Codecov by Sentry. |
Lyndon-Li
force-pushed
the
issue-fix-8032
branch
from
78c087d to
deff3c8
Compare
pkg/cmd/cli/nodeagent/server.go
Outdated
| return | ||
| } | ||
|
|
||
| if configs == nil { | ||
| s.logger.Infof("Node agent configs are not found") | ||
| s.logger.WithField("configMap", s.config.nodeAgentConfig).Infof("Node agent configs are not found") |
There was a problem hiding this comment.
NIT: Node agent config data is empty/nil
There was a problem hiding this comment.
As you can see from nodeagent.GetConfigs, configs==nil && err ==nil means the configMap with the name specified by s.config.nodeAgentConfig is not found, it doesn't mean the configMap data is empty.
There was a problem hiding this comment.
The logic seems sound. But IMO its just that when a user sees a not found log they directly interpret it as a Not Found error, implying that the user specified a config map but velero was unable to fetch/find the config map, but this is not the case here, velero is able to find the config map (as err == nil) but the config map does not consist of any node agent configs.
There was a problem hiding this comment.
@shubham-pampattiwar
I got what you mean. But I think it is not a log problem:
- Now users need to specific the configMap name explicitly, if the configMap is not found, it should be treated as an error by
nodeagent.GetConfigsas same as other errors (since the configMap is specified by users, we should always assume that the configMap exists) - Then node-agent server will print a warning log and ignore the
specified node-agent-configparameter
Therefore, I changed the logic a little bit, pls review it.
Signed-off-by: Lyndon-Li <[email protected]>
Lyndon-Li
force-pushed
the
issue-fix-8032
branch
from
deff3c8 to
8cf1749
Compare
Fix issue #8032, make node-agent configMap name configurable