Bug 823965 (JON3-45, PRODMGT-131, PRODMGT-417)

In a JBoss ON agent environment, the log4j.xml and agent-configuration.xml files may be customized to meet audit requirements, log file naming and placement requirements, perform silent installs, provide advanced configuration options, or provide a customized agent configuration template. These customizations must be maintained even through agent updates and upgrades or by a user performing a --cleanconfig or --fullcleanconfig command.

If these file are not maintained through agent updates/upgrades, the result is that an agent reinstall, --cleanconfig, --fullcleanconfig or change in user or system account would result in an agent reconfiguration using invalid options and the user's custom configuration being dropped.

Larry O'Leary <loleary> made a comment on jira PRODMGT-417

ACK'd by GSS product liaison and ready for review by product management during next major/minor release planning phase.

This needs to be discussed more.

I think retaining the old .xml might have to bad side effects that we haven't thought about.

For example, what happens if a new agent is shipped with a new agent-config.xml that adds new settings? If we blindly overwrite the new .xml with the old .xml and reset the config to use that old .xml, its possible the new agent will break.

(In reply to John Mazzitelli from comment #4)
> This needs to be discussed more.
> 
> I think retaining the old .xml might have to bad side effects that we
> haven't thought about.
> 
> For example, what happens if a new agent is shipped with a new
> agent-config.xml that adds new settings? If we blindly overwrite the new
> .xml with the old .xml and reset the config to use that old .xml, its
> possible the new agent will break.

Same goes with log4j.xml - what if we add new log4j.xml categories? We would need to somehow combine the old with the new.

As with all quality software, this is handled by the code. If a new property is introduced which has not been explicitly defined, then a reasonable default is used. 

In other words, the expectation is that I can take a brand new agent without any previous configuration and delete conf/agent-configuration.xml and I expect the agent to start-up without issue assuming that I provide it with a server host and port to communicate with. There should be no other configuration that is required from the user and when new properties are introduced, the code simply hands them a suitable default value if they are returned as null (i.e. undefined).

I would suggest that the new configuration be kept in agent-configuration.xml.new and log4j.xml.new so that we at least provide the user with the updated defaults in case they want to manually merge them.

The alternative is to perform the configuration merge ourselves but this is risky as we can not be certain to what properties the user has actually modified or added. In which case a three-way diff would be necessary but still error prone. Basically:

*   if property value from original agent-configuration.xml (based on the product version) is the same in the current configuration and new configuration, no change is necessary
*   if property value from original agent-configuration.xml (based on product version) is the same as the new configuration, then use value as is from current configuration
*   if property value from current configuration is the same as the new configuration, then use value as is from current configuration
*   if property value from original agent-configuration.xml (based on the product version) is the same in the current configuration, use value from new configuration
*   if property value from original agent-configuration.xml (based on the product version) is different then the current configuration and value in current configuration is different then new configuration, then use value from current configuration and report a WARNING to the user to have them manually investigate

git commit to master: df3f406 - Maintain existing agent-configuration.xml and log4j.xml files after agent auto-update. The new configuration will be kept in agent-configuration.xml.new and log4j.xml.new so that we at least provide the user with the updated defaults in case they want to manually merge them.

Heiko Rupp <hrupp> updated the status of jira JON3-45 to Resolved

Moving to ON_QA as available to test with brew build of DR01: https://brewweb.devel.redhat.com//buildinfo?buildID=373993

If the files 'log4j.xml' and 'agent-configuration.xml' are changed by the user, they are preserved during upgrade and the new configuration files are stored with names 'log4j.xml.new' and 'agent-configuration.xml.new' as expected.

However, the same outcome is visible even if neither of files is changed by the user before the upgrade, which is not correct behaviour, as specified in this test case - https://tcms.engineering.redhat.com/case/372401/?from_plan=13965. This issue is described in BZ 1101128.

So I just wanted to discuss what is the scope of this particular BZ? I suggest marking it as verified and leaving the rest for the other BZ (1101128).

(In reply to Jan Bednarik from comment #13)
> So I just wanted to discuss what is the scope of this particular BZ? I
> suggest marking it as verified and leaving the rest for the other BZ
> (1101128).

I think we should mark this as failed as it is a feature request and the test-case that failed was identified in the requirements.

Additionally, I see that BZ-1101128 was created upstream. Product related issues -- especially feature and enhancements -- should be tracked in the product bug tracker.

(In reply to Jan Bednarik from comment #13)
> If the files 'log4j.xml' and 'agent-configuration.xml' are changed by the
> user, they are preserved during upgrade and the new configuration files are
> stored with names 'log4j.xml.new' and 'agent-configuration.xml.new' as
> expected.
> 
> However, the same outcome is visible even if neither of files is changed by
> the user before the upgrade, which is not correct behaviour, as specified in
> this test case -
> https://tcms.engineering.redhat.com/case/372401/?from_plan=13965. This issue
> is described in BZ 1101128.
> 
> So I just wanted to discuss what is the scope of this particular BZ? I
> suggest marking it as verified and leaving the rest for the other BZ
> (1101128).

Just so I'm clear - the problem that is considered not correct behavior only occurs when the files are not changed, correct?

So, more specifically, this means that when agent-configuration is not changed by the user, both agent-configuration.xml and agent-configuration.xml.new exist and are identical in content. Correct?

Same thing with log4j.xml and log4j.xml.new (they both exist, and are both identical). Correct?

(In reply to John Mazzitelli from comment #15)
> Just so I'm clear - the problem that is considered not correct behavior only
> occurs when the files are not changed, correct?

Yes, we are only considering the case were the user did not change either of agent-configuration.xml and log4j.xml files.

> So, more specifically, this means that when agent-configuration is not
> changed by the user, both agent-configuration.xml and
> agent-configuration.xml.new exist and are identical in content. Correct?

Actually, in this particular case, when I upgrade from JON 3.2.0 GA to JON 3.3.0 DR01, both files exist but they are not identical in content because JON 3.3.0 intorduces new version of 'agent-configuration.xml' with some differencies. Still, I think that if the user did not change the original file, it should not be preserved and there should be only one file present after the upgrade - 'agent-configuration.xml' - with the new content. Now this file with new content, which JON 3.3.0 introduces, is named as 'agent-configuration.xml.new' and thus the server does not use it, right?

> Same thing with log4j.xml and log4j.xml.new (they both exist, and are both
> identical). Correct?

The same applies for 'log4j.xml'

I am attaching 'agent-configuration.xml' and 'agent-configuration.xml.new' files, which are present in ~/rhq-agent/conf direcotry after the upgrade. 'agent-configuration.xml' was not changed before the upgrade.

Created attachment 924802 [details]
agent-configuration.xml - before upgrade

Created attachment 924803 [details]
agent-configuration.xml.new - after upgrade

(In reply to Jan Bednarik from comment #16)
> Actually, in this particular case, when I upgrade from JON 3.2.0 GA to JON
> 3.3.0 DR01, both files exist but they are not identical in content because
> JON 3.3.0 intorduces new version of 'agent-configuration.xml' with some
> differencies. Still, I think that if the user did not change the original
> file, it should not be preserved

How do we know if the user did not change the file?

Ok, so it looks like there is a bug here.

Replication procedure (using pseudo-version numbers and config to make it clear what the problem is):

1) Install version 1.0.
2) Notice agent-configuration.xml has property "foo=bar"
3) Make no changes to the .xml.
4) Upgrade to version 2.0 where it introduces a new config property "a=b" in its agent-configuration.xml

At this point, agent-configuration.xml should look identical to agent-configuration.xml.new - that is, the agent's configuration should be what the new agent wants it to be - it should have foo=bar and a=b properties.

The bug is that the new upgraded agent only has foo=bar in agent-configuration.xml which is bad because, since the user never made changes, it shoudl always get the latest-n-greated config .xml from the upgraded agent.

I am really starting to wonder if we need to back out this change.

The main issue here is - how do we know if the user did not change the file?

Consider the case that just happened - we release a new agent where we, ourselves, release changes (additions, modifications, deletions) of configuration settings in agent-configuration.xml. Our new agent-configuration.xml does NOT match any older agent versions of agent-configuration.xml.

Now, how would we know the user did or did not change the OLD configuration file? Today we have no knowledge of what OLDER agent-configuration.xml files looked like. We don't store the old agent config xml files anywhere, we don't store old hash codes of them. We just have no knowledge of what the old agents' agent-configuration.xml looked like.And if we don't know what the old config file looked like, we don't know if the current version (prior to updating) is a changed file or the same old file!

Right now, I don't know of a way to support this feature request. For the future we can, if we start storing old configuration files, or the hashcodes of those old files, somewhere. But right now, we don't have that. We could go back, get the hashcodes of all the old versions' config files and hardcode that into the agent upgrade stuff. That would be one way to do it. We would just have to remember to keep adding those hashcodes as new releases are produced.

I don't think we have to back the feature out. We just have to live with the fact that an upgraded agent always keeps its old configuration and installs a new copy from the new version with the .new extension. If of course we determine that the .new version is the exact same as the old, no need to store it.

Otherwise, always have a .new. This might not always be desirable but is better then wiping out the user provided configuration. A future enhancement, if needed, could be to properly determine or do a three-way diff to figure out what changed from the orig, to the old, to the new.

The important thing for this feature is that we don't overwrite the existing agent-configuration.xml or log4j.xml files. This means that any configuration changes in the agent will need to always use reasonable defaults outside of the configuration file. In other words, if not defined there, use default value _y_.

(In reply to Larry O'Leary from comment #22)
> I don't think we have to back the feature out. We just have to live with the
> fact that an upgraded agent always keeps its old configuration and installs
> a new copy from the new version with the .new extension.

OK, that is fair. We should just document (in release notes or whereever is appropriate) that the upgraded agent will always keep the old configuration if it is at all different than the new (whether or not the user changed the config or its the past default file).

> If of course we determine that the .new version is the exact same as
> the old, no need to store it.

I just did a quick test to verify that this is what it does. If the files are identical between old and new, you don't get a .new file or anything like that.

> always use reasonable
> defaults outside of the configuration file. In other words, if not defined
> there, use default value _y_.

We do do this. Everything has reasonable defaults.

mfoley user <mfoley> updated the status of jira JON3-45 to Reopened

No further changes needed; please refer to comment 22 and comment 23.

Moving to VERIFIED.

I tested this issue by upgrading both from JON 3.1.2 -> JON 3.3.0 ER02 and JON 3.2.0 -> JON 3.3.0 ER02. Both test cases meet the new criteria specified in comment 22 and comment 23 - the old configuration files are preserved and their new counterparts are saved as *.new. In both cases the new configuration files were different from the old ones so after the upgrade all 4 files (agent-configuration.xml, agent-configuration.xml.new, log4j.xml, log4j.xml.new) are present in ~/rhq-agent/conf/ directory.

mfoley user <mfoley> updated the status of jira JON3-45 to Resolved