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

Provide ldap_uri in Custodia uninstaller #1776

Closed
wants to merge 1 commit into from

Conversation

tiran
Copy link
Member

@tiran tiran commented Apr 4, 2018

Without ldap_uri, IPAKEMKeys parses /etc/ipa/default.conf. During
uninstallation, the file may no longer contain ldap_uri. This workaround
is required for test case
test_replica_promotion.py::TestReplicaPromotionLevel0::test_promotion_disabled

Fixes: https://pagure.io/freeipa/issue/7474
Co-authored-by: Felipe Barreto fbarreto@redhat.com
Signed-off-by: Christian Heimes cheimes@redhat.com

Alternative solution for #1772

@tiran tiran requested a review from felipevolpone April 4, 2018 15:55
@felipevolpone felipevolpone added the re-run Trigger a new run of PR-CI label Apr 4, 2018
@freeipa-pr-ci2 freeipa-pr-ci2 removed the re-run Trigger a new run of PR-CI label Apr 4, 2018
@tiran tiran force-pushed the issue7474_custodia_ldap_uri branch from 0fdae20 to 0285760 Compare April 5, 2018 09:15
@slaykovsky slaykovsky added the re-run Trigger a new run of PR-CI label Apr 5, 2018
@freeipa-pr-ci freeipa-pr-ci removed the re-run Trigger a new run of PR-CI label Apr 5, 2018
@tiran tiran added the re-run Trigger a new run of PR-CI label Apr 6, 2018
@freeipa-pr-ci freeipa-pr-ci removed the re-run Trigger a new run of PR-CI label Apr 6, 2018
@frasertweedale frasertweedale self-assigned this Apr 9, 2018
@frasertweedale
Copy link
Contributor

Is this the agreed approach, @tiran @felipevolpone? i.e. should #1772 be closed?

@tiran tiran closed this Apr 9, 2018
@tiran tiran reopened this Apr 9, 2018
@tiran
Copy link
Member Author

tiran commented Apr 9, 2018

@frasertweedale yes, that's my plan. @felipevolpone 's patch fixes the issue, too. But his approach may also hide a bug in the configuration.

Without ldap_uri, IPAKEMKeys parses /etc/ipa/default.conf. During
uninstallation, the file may no longer contain ldap_uri. This workaround
is required for test case
test_replica_promotion.py::TestReplicaPromotionLevel0::test_promotion_disabled

Fixes: https://pagure.io/freeipa/issue/7474
Co-authored-by: Felipe Barreto <fbarreto@redhat.com>
Signed-off-by: Christian Heimes <cheimes@redhat.com>
@tiran tiran force-pushed the issue7474_custodia_ldap_uri branch from 0285760 to 21ce0de Compare April 9, 2018 06:56
@tiran tiran added the re-run Trigger a new run of PR-CI label Apr 9, 2018
@freeipa-pr-ci freeipa-pr-ci removed the re-run Trigger a new run of PR-CI label Apr 9, 2018
@tiran tiran added the re-run Trigger a new run of PR-CI label Apr 9, 2018
@freeipa-pr-ci freeipa-pr-ci removed the re-run Trigger a new run of PR-CI label Apr 9, 2018
@frasertweedale
Copy link
Contributor

Did an install and uninstallation, no worries. All the tests are passing and the code looks fine so here's my ACK.

@frasertweedale frasertweedale added the ack Pull Request approved, can be merged label Apr 10, 2018
@tiran tiran added the pushed Pull Request has already been pushed label Apr 10, 2018
@tiran
Copy link
Member Author

tiran commented Apr 10, 2018

master:

  • 9762bd1 Provide ldap_uri in Custodia uninstaller

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
Development

Successfully merging this pull request may close these issues.

6 participants