Repository navigation
NMS-10432: Dead configuration cleanup - #9008
Open
joseanesONMS wants to merge 5 commits into
Open
joseanesONMS wants to merge 5 commits into
joseanesONMS wants to merge 5 commits into
Conversation
The example only duplicated the shipped etc/threshd-configuration.xml with long-gone services (JBoss32, JBoss4) and nothing reads it: git grep finds no reference in assemblies, packaging (spec, debian), installer, docs or code. The only user was the EXAMPLE entry in WillItUnmarshalIT, removed here. The examples directory is copied as a whole, so no assembly descriptor changes. Customer copies of the file under etc/examples are never loaded.
Nothing reads the users.xml header mstation value: git grep finds only the JAXB getter/setter in config.users.Header, whose callers (UserManager) just carry the header through a save unchanged, plus test fixtures. users.xsd keeps the element as optional and documents it as deprecated, so existing users.xml files that still contain it keep validating through @ValidateUsing; the Java property stays for the same reason. A header without it is marshalled without the element.
…gone Every config file in the config-tester map still ships in etc on develop (vacuumd-configuration.xml included), except database-schema.xml, which has been loaded from the opennms-config jar since HZN-1308; its map entry, its two beans and its test go, and `config-tester database-schema.xml` now reports an unknown config name. Two other leftovers go: the lazy, never referenced snmpConfigDao bean, whose class FactoryBasedSnmpConfigDao no longer exists, and the ConfigTesterTest ignore entry for mapsadapter-configuration.xml, which is no longer shipped (git grep finds it only in the ConfigFileConstants id table and opennms.spec filters). The tester never looks at files outside its map, so customer etc directories that still hold these files are unaffected.
Properties.store() always writes a "#<date>" first line, so every install run rewrote etc/libraries.properties even when the library paths were unchanged. The installer now stores to a buffer and drops that first line, keeping the rest byte for byte (ISO-8859-1, same escaping and order), and closes the stream it previously leaked. Readers only load the file as Properties (Bootstrap, Installer) or grep for opennms.library.* lines (opennms.spec); nothing reads the comment.
marshallmassengill
left a comment
marshallmassengill
left a comment
Contributor
There was a problem hiding this comment.
I think this is good but wait for another review before merging. I don't see any issues with what's included here.
cgorantla
reviewed
Oct 22, 2026
cgorantla
left a comment
cgorantla
left a comment
Contributor
There was a problem hiding this comment.
- The PR says install -dis "no longer rewrites the file". That is not quite right: the content stays the same, but the file is still rewritten and its modification time changes. Either correct the wording in the PR description or skip the write when the content has not changed.
- testStoreWithoutTimestampKeepsPropertiesFormat builds its expected value with the same logic as the code it tests. The first test already covers the behaviour, so this is a weak test rather than a wrong one.
- filters.adoc:8 says database-schema.xml is in /opt/opennms/etc, which is wrong. The PR did not cause this, but it is a cheap docs fix.
Comment on lines
1224
to
1226
| if(!f.createNewFile()) { | ||
| LOG.warn("Could not create file: {}", f.getPath()); | ||
| } |
Contributor
There was a problem hiding this comment.
"Could not create file" warning in Installer.java:1224 now logs on every re-run, because the file already exists. Remove the createNewFile() check, since FileOutputStream creates the file anyway.
Contributor
Author
There was a problem hiding this comment.
Removed in c1232cd; Files.write creates the file when it is missing.
…hanged install -dis now compares the formatted properties with the existing file and leaves it, including its modification time, untouched when they match. The createNewFile() check is gone: it warned on every re-run because the file already exists, and Files.write creates it when it does not. The format test now asserts literal expected output instead of rebuilding it with the code under test, and a new test checks the modification time. The filters reference page no longer says database-schema.xml is in etc; it has been loaded from the opennms-config jar since HZN-1308.
Contributor
Author
|
@cgorantla Addressed in c1232cd:
The failing |
marshallmassengill
left a comment
marshallmassengill
left a comment
Contributor
There was a problem hiding this comment.
Looks like the feedback was all addressed.
marshallmassengill
approved these changes
Oct 23, 2026
marshallmassengill
left a comment
marshallmassengill
left a comment
Contributor
There was a problem hiding this comment.
I think this is ok but wait for @cgorantla to review changes from feedback.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Removes dead configuration from shipped config files and the installer. Nothing a running system uses changes.
etc/examples/threshd-configuration.xmland itsWillItUnmarshalITentryetc/examplesis never loadedusers.xmlno longer ships<mstation>;users.xsdmakes it optional and documents it as deprecated and ignored; JAXBHeaderno longer marks it requiredHeadergetter/setter touch it;UserManagerwrites back the header it readusers.xmlwith<mstation>still validates (checked with xmllint) and keeps its valuedatabase-schema.xmlentry, beans and test; deadsnmpConfigDaobean; stalemapsadapter-configuration.xmltest ignore; the filters reference page no longer placesdatabase-schema.xmlinetcdatabase-schema.xmlis loaded from the jar since HZN-1308;FactoryBasedSnmpConfigDaono longer exists; every other mapped file still shipslibraries.propertiesis written without theProperties.store()date comment, otherwise byte-identical, and is not written at all when its content would not changeopennms.specgrep ofopennms.library.*linesinstall -disleaves an unchanged file untouched, modification time included, and no longer logs "Could not create file" on re-runsVerification: config-model and config-tester unit tests (including
ConfigTester -aover the default etc) and the installer tests pass on current develop; the oldusers.xmlwith<mstation>validates against the new XSD.External References