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

Add force-join option to replica install #691

Closed
wants to merge 2 commits into from

Conversation

stlaz
Copy link
Contributor

@stlaz stlaz commented Apr 5, 2017

This patchset adds the force-join option to the replica installer. It also tries to improve the developer's experience by narrowing down the scope of originally an all-eating try-except block.

@@ -177,6 +176,9 @@ class ServerInstallInterface(ServerCertificateInstallInterface,
preserve_sssd = False
no_sssd = False

force_join = client.ClientInstallInterface.force_join
force_join = replica_install_only(force_join)
Copy link
Contributor

Choose a reason for hiding this comment

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

This bit seems to be unnecessary, since you are masking force_join in ServerMasterInstall with a constant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right. I meant to provide a more general approach but it probably does not make much sense here.

@@ -595,6 +595,8 @@ class ServerReplicaInstall(ServerReplicaInstallInterface):
subject_base = None
ca_subject = None

force_join = client.ClientInstallInterface.force_join
Copy link
Contributor

Choose a reason for hiding this comment

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

I meant this bit as well, force_join is already present in ServerReplicaInstall through inheritance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bah, I must be broken today, you're right, of course.

When installing client from inside replica installation on DL1,
it's possible that the client installation would fail and recommend
using --force-join option which is not available in replica installer.
Add the option there.

https://pagure.io/freeipa/issue/6183
The exception handling of client install inside replica installation
was rather promiscuous, hungrily eating any possible exception thrown
at it. Scoped down the try-except block and reduced its promiscuity.
This change should improve the future development experience debugging
this part of the code.

https://pagure.io/freeipa/issue/6183
@tkrizek tkrizek added the ack Pull Request approved, can be merged label Apr 12, 2017
@tkrizek
Copy link
Contributor

tkrizek commented Apr 12, 2017

master:

  • 87051f5 Add the force-join option to replica install
  • db84516 replicainstall: better client install exception handling

ipa-4-5:

  • 72f0ecd Add the force-join option to replica install
  • 534df55 replicainstall: better client install exception handling

@tkrizek tkrizek added the pushed Pull Request has already been pushed label Apr 12, 2017
@tkrizek tkrizek closed this Apr 12, 2017
@stlaz stlaz deleted the force_join branch July 7, 2017 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack Pull Request approved, can be merged pushed Pull Request has already been pushed
Projects
None yet
3 participants