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

Use new method in check to prevent removal of last KRA #5908

Closed
wants to merge 2 commits into from

Conversation

rcritten
Copy link
Contributor

It previously used a vault connection to determine if any
KRA servers were installed. This would fail if the last KRA
was not available.

Use server roles instead to determine if the last KRA server
is to be removed.

https://pagure.io/freeipa/issue/8397

Signed-off-by: Rob Crittenden rcritten@redhat.com

@rcritten rcritten added the ipa-4-9 Mark for backport to ipa 4.9 label Jul 20, 2021
@fcami fcami self-requested a review July 20, 2021 03:31
@fcami fcami self-assigned this Jul 20, 2021
@rcritten rcritten force-pushed the issue_8397 branch 5 times, most recently from efd8298 to 03bce90 Compare July 21, 2021 12:42
Copy link
Contributor

@fcami fcami left a comment

Choose a reason for hiding this comment

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

hi @rcritten thanks for the contribution, could you please have a look at my comments?

ipatests/test_integration/test_installation.py Outdated Show resolved Hide resolved
ipatests/test_integration/test_installation.py Outdated Show resolved Hide resolved
@fcami
Copy link
Contributor

fcami commented Jul 21, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

It previously used a vault connection to determine if any
KRA servers were installed. This would fail if the last KRA
was not available.

Use server roles instead to determine if the last KRA server
is to be removed.

https://pagure.io/freeipa/issue/8397

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
@fcami
Copy link
Contributor

fcami commented Jul 21, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rcritten
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Use the new role-based mechanism, one that doesn't rely
on direct communication to the server, to determine whether
the server being removed by `ipa server-del` contains the
last KRA server.

https://pagure.io/freeipa/issue/8397

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
Copy link
Contributor

@fcami fcami left a comment

Choose a reason for hiding this comment

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

ACK for when CI passes. Please post the link to a green CI run and remove the temp commit, I'll add the ACK.

@fcami
Copy link
Contributor

fcami commented Jul 22, 2021

Green CI run

@fcami
Copy link
Contributor

fcami commented Jul 22, 2021

@rcritten from my PoV you can remove the temp commit to start gating instead.

@rcritten rcritten added the re-run Trigger a new run of PR-CI label Jul 22, 2021
@freeipa-pr-ci freeipa-pr-ci removed the re-run Trigger a new run of PR-CI label Jul 22, 2021
Copy link
Contributor

@fcami fcami left a comment

Choose a reason for hiding this comment

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

ACK

@fcami fcami added the ack Pull Request approved, can be merged label Jul 22, 2021
@rcritten rcritten added the pushed Pull Request has already been pushed label Jul 22, 2021
@rcritten
Copy link
Contributor Author

master:

  • 10bd66d Use new method in check to prevent removal of last KRA
  • 2097776 ipatests: test removing last KRA when it is not running

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 ipa-4-9 Mark for backport to ipa 4.9 pushed Pull Request has already been pushed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants