Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

aq pxeswitch: allow to configure aii-installfe cdburl #114

Closed
wants to merge 1 commit into from

Conversation

jouvin
Copy link
Contributor

@jouvin jouvin commented Jul 14, 2018

This helps with sharing an AII server between several sources, in particular between SCDB and Aquilon.

Fixes #113 (#113)

Requires quattor/aii#291 (aii-installfe counterpart) to be useful.

@@ -155,6 +155,8 @@ plenarydir = %(cfgdir)s/plenary
service = %(user)s
keytab = /var/spool/keytabs/%(service)s
installfe_user = %(user)s
# Uncomment and define installfe_cdburl if you want Aquilon to manage cdburl
#installfe_cdburl = Aquilon profiles URL
Copy link

@urbonegi urbonegi Jul 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point of has value is that it allows to keep the empty variable namespace without a need to comment it out. Suggest to do this like it:
# Define installfe_cdburl if you want Aquilon to manage cdburl
installfe_cdburl =

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was what I did initially but I got an error with an empty value... I should probably retest it but as I'm leaving tomorrow evening for one month, I am not sure I'll be able to do it before... Will do my best!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please share the error, but i tested the has_value plenty of times.. weird...

@jouvin
Copy link
Contributor Author

jouvin commented Jul 26, 2018

I confirm that having an empty value works like a charm! Not sure what I made wrong in my initial tests...

@jrha
Copy link
Member

jrha commented Jul 26, 2018

Hang on, this assumes you only have one cdburl across your entire plant.
Would this not be better looked up against a service? i.e. in the same way there is bootserver, could we have profileserver?

In case it isn't obvious why this might be useful, consider very restricted environments such as clusters inside a private network which may use reverse proxies to provide monitored access to external resources.

OTOH it should only affect shared AII servers and not using currently does the right thing, so it may not matter.

@jouvin
Copy link
Contributor Author

jouvin commented Jul 26, 2018

@jrha I don't understand your point. It assumes only that you have one cdburl per broker. You can set the cdburl differently on each broker.

@jrha
Copy link
Member

jrha commented Jul 26, 2018

You may have one broker, but multiple URLs that serve profiles from it: mirrors, reverse proxies etc.

As I said, I don't think it matters that much, you can configure the AII server however you want unless you use the override for a shared server.

@jouvin
Copy link
Contributor Author

jouvin commented Jul 26, 2018

I understand, I didn't have this configuration in mind... But as you say, if you are using the configuration you mention, probably the price to pay is that sharing the AII server with SCDB is more complicated... A service instance based approach may bring a more general solution but looks to me as much more complex as there is no way AFAIK to define data to be passed to the service by the broker (not in a template), as in this particular case it is the broker which is building the aii-installfe command.

If it is acceptable for you, I'd appreciate to see this PR merged to make SCDB transition easier.

@jrha
Copy link
Member

jrha commented Jul 27, 2018

Sure, I don't really have a problem with this PR, just wanted to flag it!

@XaF
Copy link

XaF commented Jul 27, 2018

Can this be rebased on the current master please?
The tests used internally in Morgan Stanley changed a bit and requires commits that have been included in the last master. Thank you!

@jouvin
Copy link
Contributor Author

jouvin commented Jul 28, 2018

Done

@XaF
Copy link

XaF commented Aug 15, 2018

@jouvin so actually, it was not working but because of a change in the version of the panc compiler (an error message that changed). With the new master it should work. Could you please rebase ? I will post that same message in other pull requests sharing the same issue.

Sorry for asking the rebase a second time. Looking at the rest of the tests, this should be fine this time, and I'll try to merge it asap :)

- This helps with sharing an AII server between several sources,
in particular between SCDB and Aquilon
- Option installfe_cdburl is undefined by default (cdburl not
managed by Aquilon)

Fixes quattor#113 (quattor#113)

Change-Id: I80e8711f267863c44ecd5e6b8661ee926aab7628
@jouvin
Copy link
Contributor Author

jouvin commented Aug 16, 2018

@XaF No problem, done!

@XaF
Copy link

XaF commented Sep 13, 2018

Merged in 235644e

@XaF XaF closed this Sep 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants