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

[v8.0] feat: add documentation to setup pilots with tokens #7176

Merged

Conversation

aldbr
Copy link
Contributor

@aldbr aldbr commented Aug 21, 2023

Related to #7123

BEGINRELEASENOTES
*docs
NEW: add documentation about setting up DIRAC to submit pilots with tokens
ENDRELEASENOTES

@aldbr aldbr requested a review from andresailer as a code owner August 21, 2023 16:54
@DIRACGridBot DIRACGridBot added the alsoTargeting:integration Cherry pick this PR to integration after merge label Aug 21, 2023
@aldbr aldbr force-pushed the rel-v8r0_FEAT_DocToSetupPilotWithTokens branch from b29bff5 to cf3c0c3 Compare August 22, 2023 07:33
Copy link
Contributor

@andresailer andresailer left a comment

Choose a reason for hiding this comment

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

In principle almost all back ticks should probably be double backticks.
(Single backticks are references, double backticks are "code")

But is fine to me, except maybe for some clarifications.

Also maybe @marianne013 or @sfayer want to read through this as they have tried it?

docs/source/AdministratorGuide/HowTo/pilotsWithTokens.rst Outdated Show resolved Hide resolved
docs/source/AdministratorGuide/HowTo/pilotsWithTokens.rst Outdated Show resolved Hide resolved
docs/source/AdministratorGuide/HowTo/pilotsWithTokens.rst Outdated Show resolved Hide resolved
@aldbr aldbr force-pushed the rel-v8r0_FEAT_DocToSetupPilotWithTokens branch from cf3c0c3 to eed94af Compare August 22, 2023 11:17
@marianne013
Copy link
Contributor

This needs a warning that this is not multi-VO compatible, i.e. for a given CE either all or none of the VOs using the CE have to use tokens.

@aldbr aldbr force-pushed the rel-v8r0_FEAT_DocToSetupPilotWithTokens branch from eed94af to a16c43a Compare September 18, 2023 12:46
@aldbr
Copy link
Contributor Author

aldbr commented Sep 18, 2023

given CE either all or none of the VOs using the CE have to use tokens.

I added a warning.
I am planning to take a look at the multi-VO case this week 🤞

@marianne013
Copy link
Contributor

Did we ever agree on any strategy for multi-VO ? I guess we could have a property of the CE along the line of X509VOs = ...., then as VOs transition to tokens there would be less and less of them in that list until the've all transitioned and it's empty.

@aldbr
Copy link
Contributor Author

aldbr commented Sep 18, 2023

Did we ever agree on any strategy for multi-VO ?

Not really actually 😅

I guess we could have a property of the CE along the line of X509VOs = ...., then as VOs transition to tokens there would be less and less of them in that list until the've all transitioned and it's empty.

But then you would have to manually update this list right?

What about having the logic in the SiteDirector. It would submit pilots with tokens under two conditions:

  • (the current one) the CE is able to receive tokens. Validation: Tag = Token should be included in the CE parameters.
  • (a new one) the VO is able to produce tokens. Validation: IdProvider option is set in /Registry/VO/<VO name>/.

If one of these 2 conditions is not satisfied, then the Site Director submits pilots with proxies.

What do you think @marianne013 ?

@marianne013
Copy link
Contributor

What if we have a VO and some sites use tokens and other don't ? I believe that is still the case for CMS (not that they are using DIRAC, but if CMS can't convince all their sites to update, what chance do I have ?) The transition will be a mess one way or the other. There is certainly something to be said for not trying tokens for a VO that doesn't have an non-X509 id provider.

@fstagni
Copy link
Contributor

fstagni commented Sep 18, 2023

The token tag is per-CE. It basically says "this CE accepts tokens". I think your case might be that a certain CE might accept a token from VO "A" and proxy from VO "B".

@marianne013
Copy link
Contributor

The token tag is per-CE. It basically says "this CE accepts tokens". I think your case might be that a certain CE might accept a token from VO "A" and proxy from VO "B".

We assume that will be the most common scenario, not all VOs will get their tokens ready at the same time.

@aldbr
Copy link
Contributor Author

aldbr commented Sep 18, 2023

With the solution I proposed:

  • VO "A" would be able to submit pilots with tokens
  • VO "B", which is not yet ready, would not set the IdProvider parameter and would still be able to submit with proxies to the same CE (SiteDirector would see that the CE is ready for tokens, but not the VO).

The only problem I see with the solution I proposed is the transition period. Here is an example:

  • VO "A" has convinced Site1, Site2 to accept their tokens. CEs from Site1, Site2 would have Tag = Token.
  • VO "B" is transitioning to tokens. Site1 accepts their tokens but Site2 needs some time to adjust their configuration (for some obscure reasons). In such a case, the Site Director would try to submit pilots with tokens to Site2, which would not be accepted.

Do you think it would be a problem?

@sfayer
Copy link
Member

sfayer commented Oct 5, 2023

The only problem I see with the solution I proposed is the transition period....
Do you think it would be a problem?

Hi,

Unfortunately I think this is likely to be a problem: It's likely that we will want to switch VOs over to tokens one at a time, test it on a small number of CE/Sites for a while and then scale it up to other sites (e.g. a long transition period). Would it be possible to add a setting or something to allow us to accommodate that?

Regards,
Simon

@aldbr aldbr force-pushed the rel-v8r0_FEAT_DocToSetupPilotWithTokens branch from a16c43a to b3590a0 Compare October 19, 2023 06:37
@aldbr
Copy link
Contributor Author

aldbr commented Oct 19, 2023

This PR can be merged once #7208 is merged.
I added some documentation for multi-VO.

@fstagni fstagni merged commit 9649f6e into DIRACGrid:rel-v8r0 Oct 20, 2023
16 checks passed
@DIRACGridBot DIRACGridBot added the sweep:done All sweeping actions have been done for this PR label Oct 20, 2023
DIRACGridBot pushed a commit to DIRACGridBot/DIRAC that referenced this pull request Oct 20, 2023
@DIRACGridBot
Copy link

Sweep summary

Sweep ran in https://github.com/DIRACGrid/DIRAC/actions/runs/6582746336

Successful:

  • integration

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
alsoTargeting:integration Cherry pick this PR to integration after merge sweep:done All sweeping actions have been done for this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants