Bug #2112: Update from 2.3.0 to 2.3.4 doesn't add shared files folder - Rudder - Issue Tracker

Actions

Copy link

Bug #2112

closed

Update from 2.3.0 to 2.3.4 doesn't add shared files folder

Added by Nicolas PERRON over 12 years ago. Updated about 9 years ago.

Status:

Released

Priority:

Assignee:

Nicolas PERRON

Category:

Packaging

Target version:

2.3.5

Pull Request:

Severity:

UX impact:

User visibility:

Effort required:

Priority:

Name check:

Fix check:

Regression:

Description

After and update from 2.3.0:
- /var/rudder/configuration-repository/shared-files folder is not created
- "rudder.dir.shared.files.folder=/var/rudder/configuration-repository/shared-files" is not added to the local rudder-web.properties configuration file

Actions

Copy link

Updated by Nicolas PERRON over 12 years ago

Subject changed from Update from 2.3.0 to 2.3.4 doesn't add share file folder to Update from 2.3.0 to 2.3.4 doesn't add shared files folder
Description updated (diff)

Actions

Copy link

Updated by Nicolas PERRON over 12 years ago

Status changed from New to Pending technical review
% Done changed from 0 to 100

Applied in changeset commit:3998177f9bff5fb51024bd0ae3c070018cdeab0d.

Actions

Copy link

Updated by Jonathan CLARKE over 12 years ago

Status changed from Pending technical review to 2

After reviewing the commit, this fix doesn't seem complete to me, and is weirdly mixed in with the login for moving policy-templates from /var/rudder/ to /var/rudder/configuration-repository.

The main problem that I see is that if we're not upgrading from 2.3.0 (ie, /var/rudder/configuration-repository doesn't exist), we will never create the shared-files directory.

Can you please review this fix to:

Separate this upgrade logic from the existing logic for moving policy-templates from /var/rudder/ to /var/rudder/configuration-repository (these are 2 separate evolutions, and should not be mixed, especially as they are easy to handle separately)
Make sure that the directories we want to create are always created

Thanks.

Actions

Copy link

Updated by Nicolas PERRON over 12 years ago

Status changed from 2 to Pending technical review

Applied in changeset commit:db599d0158b5e3288f20d9ca41ca5fad4d549c36.

Actions

Copy link

Updated by Jonathan CLARKE over 12 years ago

Status changed from Pending technical review to Discussion

This commit is an improvement, thanks.

However, some things are still not reliably implemented:

You test for the existance of /var/rudder/configuration-repository but the directory you're creating is actually /var/rudder/configuration-repository/shared-files - so if /var/rudder/configuration-repository exists but not /var/rudder/configuration-repository/shared-files, you will never create it! The simpler approach is the best here: test what you want to create.
You test for the existance of /var/rudder/configuration-repository/.git, but don't create it if it doesn't exist. This needs a bit more work.

I suggest we discuss this together tomorrow.

Actions

Copy link

Updated by Nicolas PERRON over 12 years ago

However, some things are still not reliably implemented:

You test for the existance of /var/rudder/configuration-repository but the directory you're creating is actually /var/rudder/configuration-repository/shared-files - so if /var/rudder/configuration-repository exists but not /var/rudder/configuration-repository/shared-files, you will never create it! The simpler approach is the best here: test what you want to create.

You're right, it's simply a typo ! We don't need to check /var/rudder/configuration-repository which is created at the first install or in case of migration.