2:31 a.m.
Please find the patch with fixed for review comments given by Endi for
the patches 74,75,76,77,78 as part of the basic CI test framework setup.
The comments addressed are :
--Rename pki/READ_ME_TESTS to pki/tests/dogtag/README
-- Create sample configuration files for all the config files specified
in README.
-- Specify all the steps required to setup an environment for running
the java tests in eclipse.
-- rename the task name for tests in the beaker-job.xml.template
to /CoreOS/dogtag/PKI_TEST_USER_ID.
Patch 79 has to be applied after applying patches 74,75,76,77,78.
All the patches, on ACK, will be pushed to master which closes tickets
657,722,723,724,725,785 in the 10.1 milestone.
Thanks,
Abhishek
1:27 p.m.
On 11/9/2013 2:31 AM, Abhishek Koneru wrote:
Please find the patch with fixed for review comments given by Endi
for
the patches 74,75,76,77,78 as part of the basic CI test framework setup.
The comments addressed are :
--Rename pki/READ_ME_TESTS to pki/tests/dogtag/README
-- Create sample configuration files for all the config files specified
in README.
-- Specify all the steps required to setup an environment for running
the java tests in eclipse.
-- rename the task name for tests in the beaker-job.xml.template
to /CoreOS/dogtag/PKI_TEST_USER_ID.
Patch 79 has to be applied after applying patches 74,75,76,77,78.
All the patches, on ACK, will be pushed to master which closes tickets
657,722,723,724,725,785 in the 10.1 milestone.
Thanks,
Abhishek
Thanks for the fixes. Just to clarify, I reviewed mostly patches #75,
#76 (excluding JUnit tests), #77, and #79.
As discussed before, the Beaker test cases (patch #74), JUnit test cases
(patch #76), and Jenkins setup (patch #78) will be reviewed separately
afterwards. The Beaker test execution indicates there are quite a few
failures.
Based on the initial review, there are a number of issues that need
further discussions. Please send the list whenever it's ready.
For now all of the patches are ACK'd. There are just a few more minor
issues below. Once they are fixed they can be pushed.
1. In the README section 1.1.1 let's use pki-tomcat as the default DS
instance name to match Dogtag's default instance name:
slapd.ServerIdentifier=pki-tomcat
2. The following line after setup-ds.pl is not indented correctly:
Then a CA instance ...
3. The beakerjob.rhcs.xml.template still has 'rhcs' in the file name.
Can it be renamed to beakerjob.dogtag.xml.template or just
beakerjob.xml.template?
4. In the client.conf right now all parameters are commented out. I
think we should provide a default authentication, probably the
username/password auth. So the HUB_URL, AUTH_METHOD=password, USERNAME,
and PASSWORD should be uncommented by default. The README should be
updated too.
5. In the README section 1.2.2.1 the "User-ID_for_personalization" can
we replaced with "unique_label", or something like that.
6. The "Job_xml_config_file" could be replaced with
"beaker_job_config"
and mention that the file is available at conf/beaker-job.cfg.
7. There's also an old path in that section:
pki/tests/beaker/rhcs/Makefile.
8. The example in compose_function still has reference to internal
machine and it uses a wrong parameter name. It's better to remove this
example since we already provide repository.cfg
9. The Create_repo_after_build() creates repos for SOURCES and SPECS
folders. These are probably unnecessary since they don't contain RPM files.
Please add the followings to the list of issues that need further
discussions:
10. Copying into /tmp and giving everybody a read access might not be a
safe way to pass ca_admin_cert.p12, but for test this is probably ok. A
better way is probably to copy into the test user's home directory and
change the file ownership to the test user.
cp /root/.dogtag/pki-tomcat/ca_admin_cert.p12
$TEST_HOME/.dogtag/pki-tomcat/ca_admin_cert.p12
chown $TEST_USER.$TEST_GROUP
$TEST_HOME/.dogtag/pki-tomcat/ca_admin_cert.p12
11. The test user can create the NSS DB in the user's home directory too
(e.g. ~/.dogtag/nssdb). This will require customizing test.cfg though.
mkdir ~/.dogtag/nssdb
certutil -N -d ~/.dogtag/nssdb
--
Endi S. Dewata
2:19 p.m.
Thanks alo Endi. All the comments are addressed.
The patches are pushed to master.
The tickets that will be closed are 657,722,723,724,725,785
--Abhishek
----- Original Message -----
From: "Endi Sukma Dewata" <edewata(a)redhat.com>
To: "Abhishek Koneru" <akoneru(a)redhat.com>
Cc: "pki-devel" <pki-devel(a)redhat.com>
Sent: Sunday, November 10, 2013 2:27:28 PM
Subject: Re: [Pki-devel] [PATCH] 79 Fixes for review comments on patches 74, 75, 76, 77,
78 #CI test framework
On 11/9/2013 2:31 AM, Abhishek Koneru wrote:
Please find the patch with fixed for review comments given by Endi
for
the patches 74,75,76,77,78 as part of the basic CI test framework setup.
The comments addressed are :
--Rename pki/READ_ME_TESTS to pki/tests/dogtag/README
-- Create sample configuration files for all the config files specified
in README.
-- Specify all the steps required to setup an environment for running
the java tests in eclipse.
-- rename the task name for tests in the beaker-job.xml.template
to /CoreOS/dogtag/PKI_TEST_USER_ID.
Patch 79 has to be applied after applying patches 74,75,76,77,78.
All the patches, on ACK, will be pushed to master which closes tickets
657,722,723,724,725,785 in the 10.1 milestone.
Thanks,
Abhishek
Thanks for the fixes. Just to clarify, I reviewed mostly patches #75,
#76 (excluding JUnit tests), #77, and #79.
As discussed before, the Beaker test cases (patch #74), JUnit test cases
(patch #76), and Jenkins setup (patch #78) will be reviewed separately
afterwards. The Beaker test execution indicates there are quite a few
failures.
Based on the initial review, there are a number of issues that need
further discussions. Please send the list whenever it's ready.
For now all of the patches are ACK'd. There are just a few more minor
issues below. Once they are fixed they can be pushed.
1. In the README section 1.1.1 let's use pki-tomcat as the default DS
instance name to match Dogtag's default instance name:
slapd.ServerIdentifier=pki-tomcat
2. The following line after setup-ds.pl is not indented correctly:
Then a CA instance ...
3. The beakerjob.rhcs.xml.template still has 'rhcs' in the file name.
Can it be renamed to beakerjob.dogtag.xml.template or just
beakerjob.xml.template?
4. In the client.conf right now all parameters are commented out. I
think we should provide a default authentication, probably the
username/password auth. So the HUB_URL, AUTH_METHOD=password, USERNAME,
and PASSWORD should be uncommented by default. The README should be
updated too.
5. In the README section 1.2.2.1 the "User-ID_for_personalization" can
we replaced with "unique_label", or something like that.
6. The "Job_xml_config_file" could be replaced with
"beaker_job_config"
and mention that the file is available at conf/beaker-job.cfg.
7. There's also an old path in that section:
pki/tests/beaker/rhcs/Makefile.
8. The example in compose_function still has reference to internal
machine and it uses a wrong parameter name. It's better to remove this
example since we already provide repository.cfg
9. The Create_repo_after_build() creates repos for SOURCES and SPECS
folders. These are probably unnecessary since they don't contain RPM files.
Please add the followings to the list of issues that need further
discussions:
10. Copying into /tmp and giving everybody a read access might not be a
safe way to pass ca_admin_cert.p12, but for test this is probably ok. A
better way is probably to copy into the test user's home directory and
change the file ownership to the test user.
cp /root/.dogtag/pki-tomcat/ca_admin_cert.p12
$TEST_HOME/.dogtag/pki-tomcat/ca_admin_cert.p12
chown $TEST_USER.$TEST_GROUP
$TEST_HOME/.dogtag/pki-tomcat/ca_admin_cert.p12
11. The test user can create the NSS DB in the user's home directory too
(e.g. ~/.dogtag/nssdb). This will require customizing test.cfg though.
mkdir ~/.dogtag/nssdb
certutil -N -d ~/.dogtag/nssdb
--
Endi S. Dewata
3827
days inactive
3828
days old
2 comments
2 participants
participants (2)
-
Abhishek Koneru -
Endi Sukma Dewata