[WIP] refactor: replace hardcoded paths with constants from locationconsts.go (#3682)
This PR resolves Issue #3682 by replacing hardcoded filesystem paths with centralized constants from locationconsts.go across the pillar modules, improving code maintainability and consistency. The refactoring centralizes path management while preserving existing functionality and maintaining clear separation between EVE-managed paths and system paths.
Currently, 5 modules have been updated pending test verification (see commits for details):
-
pkg/pillar/types/locationconsts.go
-
pkg/pillar/agentlog/*
-
pkg/pillar/base/*
-
pkg/pillar/hypervisor/*
-
pkg/pillar/pidfile/*
PR dependencies
How to test and validate this PR
make test pkg/pillar
Changelog notes
This addresses technical debt by centralizing filesystem path management, making future path changes easier to implement and reducing the risk of inconsistent path usage across the codebase.
PR Backports
N/A - This is a refactoring change that improves maintainability without changing functionality.
Checklist
- [x] I've provided a proper description
- [x] I've added the proper documentation (when applicable)
- [ ] I've tested my PR on amd64 device(s)
- [ ] I've tested my PR on arm64 device(s)
- [x] I've written the test verification instructions
- [x] I've set the proper labels to this PR
@eriknordmark this will be a multi-commit PR, module by module.
@milan-zededa @christoph-zededa
There is a minor opportunity to centralize the path "/run/eve-hv-type"
https://github.com/lf-edge/eve/blob/7ab0c0b8b14650228e9ff3508a2606ba889fe8ab/pkg/pillar/base/kubevirt.go#L15-L16
https://github.com/lf-edge/eve/blob/7ab0c0b8b14650228e9ff3508a2606ba889fe8ab/pkg/pillar/hypervisor/hypervisor.go#L98
However, pkg/pillar/types/dns.go imports pkg/pillar/base for pkg/pillar/base/logobjecttypes.go
https://github.com/lf-edge/eve/blob/7ab0c0b8b14650228e9ff3508a2606ba889fe8ab/pkg/pillar/types/dns.go#L15
Because pkg/pillar/base is used in so many modules (not limited to pkg/pillar/base/logobjecttypes.go), I am inclined to leave this as-is.
Alternatively, we could just remove this function and refactor its limited use:
https://github.com/lf-edge/eve/blob/7ab0c0b8b14650228e9ff3508a2606ba889fe8ab/pkg/pillar/hypervisor/hypervisor.go#L96C1-L99C2
@jeff-zed I'm not sure if I follow the issue with the pillar/base package. Are you getting import cycle or some other build issue if you change BootTimeHypervisor to:
func BootTimeHypervisor() Hypervisor {
return bootTimeHypervisorWithHVFilePath(base.EveVirtTypeFile)
}
? Or were you considering to move EveVirtTypeFile to pillar/types?
@jeff-zed I'm not sure if I follow the issue with the pillar/base package. Are you getting import cycle or some other build issue if you change BootTimeHypervisor to:
func BootTimeHypervisor() Hypervisor { return bootTimeHypervisorWithHVFilePath(base.EveVirtTypeFile) }? Or were you considering to move EveVirtTypeFile to pillar/types?
cyclical dependency
The runner deployment script seems to be malfunctioning. @rene is currently working on fixing it.
@jeff-zed apart from the Yetus errors yet to be fixed, it looks good to me...
There is still some more to commit. When I think I am done and it is ready for full review, I will add a comment indicating that for the reviewers.
@jeff-zed apart from the Yetus errors yet to be fixed, it looks good to me...
There is still some more to commit. When I think I am done and it is ready for full review, I will add a comment indicating that for the reviewers.
Until a PR is ready for review, we usually mark it as Draft and add [WIP] to the title.
@jeff-zed apart from the Yetus errors yet to be fixed, it looks good to me...
There is still some more to commit. When I think I am done and it is ready for full review, I will add a comment indicating that for the reviewers.
Until a PR is ready for review, we usually mark it as Draft and add [WIP] to the title.
Thank you.
The runner deployment script seems to be malfunctioning. @rene is currently working on fixing it.
Thank you. This should also be helpful for EVE 15.3.0 that @eriknordmark tagged
Hi all. I noticed that pkg-deps.mk is currently ignored via .gitignore, but it needs to be versioned so CI picks up our updated dependency list.
Would you prefer I remove the *.mk (or pkg-deps.mk) entry from .gitignore, or simply force-add this one file?
There is also the question of whether edgeview should be modified to use the pkg/pillar/types/locationconst.go or not in the latest commit.
Thanks!