Add switchdev-after-networkmanager service file to k8s plugin #219
Conversation
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
|
/test-e2e-nvidia-all |
|
/cc @adrianchiris @e0ne |
zshi-redhat
force-pushed
the
issue-218
branch
from
21ece9f
to
8d105de
Compare
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
pkg/plugins/k8s/k8s_plugin.go
Outdated
| switchdevRenamingUdevScript = switchdevManifestPath + "files/switchdev-vf-link-name.sh.yaml" | ||
| switchdevManifestPath = "bindata/manifests/switchdev-config/" | ||
| switchdevUnits = switchdevManifestPath + "switchdev-units/" | ||
| switchdevBeforeNMUnitFile = switchdevUnits + "switchdev-configuration-before-NM.yaml" |
There was a problem hiding this comment.
I'm not sure that we want to have upper-case letters in file name but I won't block patch for it
There was a problem hiding this comment.
dont think its critical, although we dont use capitals for vf nor ovs.
@zshi-redhat WDYT shall we stick with lower case letters ?
There was a problem hiding this comment.
sure, updated to lowercase.
zshi-redhat
force-pushed
the
issue-218
branch
from
8d105de
to
dde16a3
Compare
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
| @@ -171,18 +187,24 @@ func (p *K8sPlugin) readSwitchdevManifest() error { | |||
| Name: "ConditionPathExists", | |||
| Value: "!/etc/ignition-machine-config-encapsulated.json", | |||
| } | |||
| switchdevService, err = service.RemoveFromService(switchdevService, conditionOpt) | |||
| switchdevBeforeNMService, err = service.RemoveFromService(switchdevBeforeNMService, conditionOpt) | |||
There was a problem hiding this comment.
I can see this condition in switchdevAfterNMService as well
I believe it should be removed as well.
pkg/plugins/k8s/k8s_plugin.go
Outdated
| @@ -232,30 +254,53 @@ func (p *K8sPlugin) readManifestFiles() error { | |||
|
|
|||
| func (p *K8sPlugin) switchdevServiceStateUpdate() error { | |||
| // Check switchdev service | |||
| swdService, err := p.serviceManager.ReadService(p.switchdevService.Path) | |||
| swdService, err := p.serviceManager.ReadService(p.switchdevBeforeNMService.Path) | |||
There was a problem hiding this comment.
we could avoid some duplicate logic here if we defined a method: bool ShouldUpdateService(Service)
and call it for both services
There was a problem hiding this comment.
Done, I used isSwitchdevServiceNeedUpdate as function name to be consistent with existing isSystemServiceNeedUpdate.
pkg/plugins/k8s/k8s_plugin.go
Outdated
| } | ||
|
|
||
| // Check switchdev run script | ||
| data, err := ioutil.ReadFile(path.Join(chroot, p.switchdevRunScript.Path)) | ||
| data, err := ioutil.ReadFile(path.Join(chroot, p.switchdevBeforeNMRunScript.Path)) |
There was a problem hiding this comment.
we could avoid some duplicate logic here if we defined a method: bool ShouldUpdateServiceScript(Service)
and call it for both service scripts
There was a problem hiding this comment.
Done, I used isSwitchdevScriptNeedUpdate as function name to be consistent with existing isSystemServiceNeedUpdate.
Signed-off-by: Zenghui Shi <[email protected]>
zshi-redhat
force-pushed
the
issue-218
branch
from
dde16a3
to
9f1bbd8
Compare
|
Thanks for your PR,
To skip the vendors CIs use one of:
|
|
/test-all |
Fixes #218
Signed-off-by: Zenghui Shi [email protected]