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

TEST: Adding krb5-libs to dependencies #218

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
4 participants
@celestian
Copy link
Collaborator

commented Mar 30, 2017

Resolves:
https://pagure.io/SSSD/sssd/issue/3353

Note:
I am not sure if this is the correct dependency which we were looking for. But it is needed anyway. If we need more don't hesitate to write me.

@lslebodn

This comment has been minimized.

Copy link
Contributor

commented Mar 30, 2017

It does not work for me:

sh$ rpm -q python-requests python2-requests python3-requests
package python-requests is not installed
python2-requests-2.13.0-1.fc26.noarch
python3-requests-2.13.0-1.fc26.noarch
checking for CMOCKA... yes
checking for uid_wrapper... yes
checking for nss_wrapper... yes
configure: error: cannot enable integration tests: python-requests not found
make: *** [Makefile:33181: intgcheck-prepare] Error 1

@celestian celestian force-pushed the celestian:t3353_master_intgcheck branch from 8cf9aad to 41e8d65 Mar 31, 2017

@celestian celestian changed the title TEST: Adding paython-requests to dependencies TEST: Adding krb5-kdc to dependencies Mar 31, 2017

@celestian celestian force-pushed the celestian:t3353_master_intgcheck branch from 41e8d65 to 4c085a3 Mar 31, 2017

@celestian celestian changed the title TEST: Adding krb5-kdc to dependencies TEST: Adding krb5-libs to dependencies Mar 31, 2017

@celestian

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 31, 2017

I see

install-deps:           success  00:01:07 ci-install-deps.log
autoreconf:             success  00:00:34 ci-autoreconf.log
DEBUG BUILD:                              ci-build-debug
configure:              failure  00:00:22 ci-build-debug/ci-configure.log
FAILURE

Is it possible to see logs?

Respectively, I tried to run the tests in our CI, but connection failed:

$ git push ci HEAD:master
ssh_exchange_identification: Connection closed by remote host
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.
@lslebodn

This comment has been minimized.

Copy link
Contributor

commented Mar 31, 2017

This change does not fix anything. Fedora/el{6,7} already install kr5 in spec file
BuildRequires: krb5-devel

@lslebodn

This comment has been minimized.

Copy link
Contributor

commented Mar 31, 2017

I think that ticket 3353 is mainly about following change

commit 0700118d8388c38b8cb28279510b206b76a3a411
Author: Jakub Hrozek <jhrozek@redhat.com>
Date:   Tue Dec 13 17:17:16 2016 +0100

    TESTS: Add integration tests for the KCM responder
    
diff --git a/contrib/ci/deps.sh b/contrib/ci/deps.sh
index c525e62e8..4467e117c 100644
--- a/contrib/ci/deps.sh
+++ b/contrib/ci/deps.sh
@@ -47,6 +47,8 @@ if [[ "$DISTRO_BRANCH" == -redhat-* ]]; then
         uid_wrapper
         python-requests
         curl-devel
+        krb5-server
+        krb5-workstation
     )
     _DEPS_LIST_SPEC=`
         sed -e 's/@PACKAGE_VERSION@/0/g' \

And it would be to check which binaries are used in added python files:
src/tests/intg/kdc.py, src/tests/intg/krb5utils.py, src/tests/intg/test_kcm.py

From top of my head: kinit

@celestian celestian force-pushed the celestian:t3353_master_intgcheck branch from 4c085a3 to 0942686 Mar 31, 2017

@celestian

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 31, 2017

@lslebodn thanks. I think I see it now. New patch pushed.

@lslebodn lslebodn force-pushed the celestian:t3353_master_intgcheck branch from 0942686 to d9cad95 Apr 5, 2017

@lslebodn

This comment has been minimized.

Copy link
Contributor

commented Apr 12, 2017

configure failed in intgcheck due to following error:

configure: error: cannot enable integration tests: krb5kdc not found

But krb5kdc was installed on system. But /usr/sbin was not probably in PATH.

http://sssd-ci.duckdns.org/logs/job/67/49/summary.html

intg: Add check for krb5-{server,workstation} binaries
The $PATH in our internal CI machines do not include sbin (at least when
running intgcheck), which forced us to manually include it there as part
of this patch. Otherwise the patches would fail in all supported CI
distros.

Resolves:
https://pagure.io/SSSD/sssd/issue/3353

@fidencio fidencio force-pushed the celestian:t3353_master_intgcheck branch from d9cad95 to f5da294 Sep 20, 2017

@fidencio

This comment has been minimized.

Copy link
Contributor

commented Sep 20, 2017

I have updated @celestian's patches in a way that sbin is also included in the PATH in our CI.

@fidencio

This comment has been minimized.

Copy link
Contributor

commented Sep 20, 2017

@jhrozek

This comment has been minimized.

Copy link
Contributor

commented Sep 21, 2017

LGTM

@jhrozek jhrozek added the Accepted label Sep 21, 2017

@@ -45,7 +45,7 @@ if [[ "$DISTRO_BRANCH" == -redhat-* ]]; then
pyldb
rpm-build
uid_wrapper
python-requests
krb5-libs

This comment has been minimized.

Copy link
@lslebodn

lslebodn Sep 25, 2017

Contributor

Could you explain this change?

SSS_INTGCHECK_REQ([HAVE_KVNO], [kvno])
SSS_INTGCHECK_REQ([HAVE_KDESTROY], [kdestroy])
SSS_INTGCHECK_REQ([HAVE_KSWITCH], [kswitch])
SSS_INTGCHECK_REQ([HAVE_KLIST], [klist])

This comment has been minimized.

Copy link
@lslebodn

lslebodn Sep 25, 2017

Contributor

I think we need to also add kadmin.local

This comment has been minimized.

Copy link
@fidencio

fidencio Sep 25, 2017

Contributor

Like the way it's done in the line below your comment or do you have something else in mind?

# As some binaries checks depend on having sbin in the
# PATH, let's add it manually here.
declare -r SBIN_PATH="/usr/local/sbin:/sbin:/usr/sbin"
export PATH=$CI_DIR:$SBIN_PATH:$PATH
export LC_ALL=C

This comment has been minimized.

Copy link
@lslebodn

lslebodn Sep 25, 2017

Contributor

That will not solve direct invocation of make intgcheck without */sbin in PATH. Moreover some distributions
have in daemon binaries in /usr/lib/mit/sbin/. We need to use pkg-config and/or krb5-config for detection.
And in should be done in configure and not in external scripts.

This comment has been minimized.

Copy link
@fidencio

fidencio Sep 25, 2017

Contributor

Okay. I'm not going to revisit this PR anytime soon, but your comment does make sense and will be considered whenever I (or someone else) revisit this PR.

Thanks for the review.

@lslebodn lslebodn added Changes requested and removed Accepted labels Sep 25, 2017

@jhrozek

This comment has been minimized.

Copy link
Contributor

commented Oct 5, 2017

Since it's unlikely that anyone will revive this PR anytime soon, let's just close it. If there are developers interested in re-submitting the work, please do so with proper attribution in a new PR.

@jhrozek jhrozek closed this Oct 5, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.