Project

General

Profile

Bug #3437

Ini installer broken in master

Added by Parag Mhashilkar almost 7 years ago. Updated almost 7 years ago.

Status:
Closed
Priority:
Normal
Assignee:
John Weigand
Category:
Ini-Installer
Target version:
Start date:
02/06/2013
Due date:
% Done:

0%

Estimated time:
First Occurred:
Occurs In:
Stakeholders:
Duration:

Description

Ini installer is badly broken in master.
Issues: * messed up imports * unsetting of PYTHONPATH in middle of the code * missing chunks of code, probably a result from incomplete merge/port from branch_v2plus

I fixed some of the issues to get it working but this needs a through investigation and testing. Fixes will be put in master_<ticketid> of this issue.


Related issues

Related to GlideinWMS - Bug #3481: ini-installer broken in branch_v2plusClosed02/15/2013

History

#1 Updated by Parag Mhashilkar almost 7 years ago

John, changes are in master_3437.
Note, I only tried installing wmscollector and factory. Other services are still untested and could have issues.

#2 Updated by John Weigand almost 7 years ago

Parag,

This appears to have occurred on Dec28 with this commit:06b50ec104accea230b205f53335a55c5a842858

The trace you showed me was from code that was removed long long time ago.

A couple questions:
1. Is there a way in git to identify all the modules affected by that commit?
- then I might be able to verify the integrity.

2. I'd think the best way to handle this is to go back to the one before it (hopefully good) and then move forward for the later changes. These were mostly for import, paths, pylint, etc.

Putting fixes in using the current master will be very chancy.

We also have a problem with anyone having created a branch off the master since that date( assuming Dec28 is the date). Disaster can strike again.

John Weigand

#3 Updated by John Weigand almost 7 years ago

The commit appears to actually have happened on 12/18 based on the log....
commit 06b50ec104accea230b205f53335a55c5a842858
Author: Doug Strain <>
Date: Tue Dec 18 16:52:30 2012 -0600

GlideinWMS Packaging 2203: Installer should add PYTHONPATH
- Took a first stab at adding the PYTHONPATH to the
-- factory.sh
-- frontend.sh
-- profile.d/condor.sh (Q&A) installer
Conflicts:
frontend/glideinFrontendInterface.py
install/glideinWMS_install
install/services/Factory.py

... but must not have been pushed until 12/28 per the repo.

I am hoping it is just those 3 files but have not found a way to tell for sure.

John Weigand

#4 Updated by John Weigand almost 7 years ago

A 'git show 06b50ec104accea230b205f53335a55c5a842858' grepping for ^diff shows these.
/frontend/glideinFrontendInterface.py
/install/glideinWMS_install
/install/services/Factory.py
/install/services/VOFrontend.py

... maybe?

But I am worried that subsequent commits may have affected others.

John Weigand

#5 Updated by John Weigand almost 7 years ago

Found a cleaner way to do ... just want to document it for future reference...

git show --pretty="format:" --name-only 06b50ec104accea230b205f53335a55c5a842858

frontend/glideinFrontendInterface.py
install/glideinWMS_install
install/services/Factory.py
install/services/VOFrontend.py

John Weigand

#6 Updated by Douglas Strain almost 7 years ago

So, a few comments. First off, I have tested master_3437. The installer works correctly (once you install python-ldap) after Parag's changes. All components installed correctly and the factory submits glideins. They are not connecting to the user pool yet, but its probably a configuration issue on my part.

Second, I don't understand John's comments above, but perhaps I can explain. The commit he referenced above was part of the change to put all the python files into a python package. This change also adds the PYTHONPATH to factory.sh/frontend.sh as part of the ini installation. The actual commit to master had a problem in that the merge pulled in the Q&A installer into master, but I deleted it out again in a later commit. I'm not exactly sure what the problem John found with that commit was.

Third, the problem that Parag solved happened because of two interacting features. One, install/services/VOFrontend.py and install/services/WMSCollector.py import each other. Two, both of those delete the PYTHONPATH. So, what happens is:
- WMSCollector is imported. It then imports VOFrontend.
- The PYTHONPATH is cleared by: os.environ["PYTHONPATH"] = ""
- VOFrontend imports WMSCollector. This fails since the PYTHONPATH was cleared.
I think Parag's patch clears this up.

#7 Updated by Burt Holzman almost 7 years ago

I think the changes in 3437 are probably OK except the bit that adds use_vofrontend_proxy, x509_proxy, and x509_gsi_dn.
Aren't these deprecated? (I think use_vofrontend_proxy got accidentally brought in on 06b50ec as part of an incorrectly resolved conflict?)

- B

#8 Updated by John Weigand almost 7 years ago

Commit in master_3437
Hash a3504d7

Modules: Factory.py

This should restore the module with these changes for the deprecated code.

Remove references to these attributes in factory options
- "use_vofrontend_proxy"
- "x509_proxy"
- "x509_gsi_dn"
.
Removed these methods and use of them:
- def use_vofrontend_proxy(self):
- def x509_proxy(self):
- def x509_gsi_dn(self):

John Weigand

#9 Updated by John Weigand almost 7 years ago

This was also causing a problem.

Removed need for ldapMonitor.
This was used to query bdii for entry points. Accessing bdii returned way
too many entries for a user to select from so it was basically commented
out back in 2010.
The import for the ldapMonitor module it used is causing failures if
the python-ldap module is not installed.

#10 Updated by John Weigand almost 7 years ago

  • Status changed from Assigned to Feedback
  • Assignee changed from John Weigand to Parag Mhashilkar

For review Parag.

#11 Updated by John Weigand almost 7 years ago

Forgot to note this.

./install/services/Factory.py
commit:a3504d7c1eff71213e2b49f61911de6e861155b1
commit:d684a94a72edfc992ae3ead89db6bab77d65d8b1

#12 Updated by John Weigand almost 7 years ago

  • Assignee changed from Parag Mhashilkar to Douglas Strain

Doug,

Parag has deferred the review to you since he has commits in here as well.

John Weigand

#13 Updated by Douglas Strain almost 7 years ago

  • Status changed from Feedback to Resolved
  • Assignee changed from Douglas Strain to John Weigand

I have reviewed it and merged it to master. It looked fine to me. There was one conflict with a pylint error I had fixed separately in master.

#14 Updated by Parag Mhashilkar almost 7 years ago

  • Status changed from Resolved to Closed


Also available in: Atom PDF