You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@bookkeeper.apache.org by GitBox <gi...@apache.org> on 2020/04/23 14:47:10 UTC

[GitHub] [bookkeeper] eolivelli opened a new pull request #2320: Test additional fix for #2219

eolivelli opened a new pull request #2320:
URL: https://github.com/apache/bookkeeper/pull/2320


   Test patch fix over #2219


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] eolivelli commented on pull request #2320: Test additional fix for #2219

Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #2320:
URL: https://github.com/apache/bookkeeper/pull/2320#issuecomment-622810541


   @jiazhai you already reviewed #2219
   I am leaning toward merging this branch to master, keeping @ravisharda as author
   
   @sijie I would like to move forward with this fix, that it is blocking the upgrade to zk 3.5.
   In Pulsar and on any other known BookKeeper application we are using ZK 3.5, it is better to have BK tests run with ZK 3.5 client)
   
   what do you think ?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] eolivelli commented on issue #2320: Test additional fix for #2219

Posted by GitBox <gi...@apache.org>.
eolivelli commented on issue #2320:
URL: https://github.com/apache/bookkeeper/pull/2320#issuecomment-618451536


   It looks like this fix does not work
   [ERROR] Tests run: 3, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 66.081 s <<< FAILURE! - in org.apache.bookkeeper.tests.integration.cluster.SimpleClusterTest
   [ERROR] test001_StartBookie on test001_StartBookie(org.apache.bookkeeper.tests.integration.cluster.SimpleClusterTest)(org.apache.bookkeeper.tests.integration.cluster.SimpleClusterTest)  Time elapsed: 60.419 s  <<< FAILURE!
   org.testcontainers.containers.ContainerLaunchException: Container startup failed
   	at org.apache.bookkeeper.tests.integration.cluster.SimpleClusterTest.test001_StartBookie(SimpleClusterTest.java:53)
   Caused by: org.rnorth.ducttape.RetryCountExceededException: Retry limit hit with exception
   	at org.apache.bookkeeper.tests.integration.cluster.SimpleClusterTest.test001_StartBookie(SimpleClusterTest.java:53)
   Caused by: org.testcontainers.containers.ContainerLaunchException: Could not create/start container
   	at org.apache.bookkeeper.tests.integration.cluster.SimpleClusterTest.test001_StartBookie(SimpleClusterTest.java:53)
   Caused by: org.testcontainers.containers.ContainerLaunchException: Timed out waiting for URL to be accessible (http://localhost:32807/heartbeat should return HTTP 200)
   	at org.apache.bookkeeper.tests.integration.cluster.SimpleClusterTest.test001_StartBookie(SimpleClusterTest.java:53)


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] eolivelli commented on a change in pull request #2320: Test additional fix for #2219

Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #2320:
URL: https://github.com/apache/bookkeeper/pull/2320#discussion_r418724124



##########
File path: tests/docker-images/current-version-image/Dockerfile
##########
@@ -36,9 +36,14 @@ ENV JAVA_HOME=/usr/lib/jvm/jre-1.8.0
 RUN set -x \
     && adduser "${BK_USER}" \
     && yum install -y epel-release \
-    && yum install -y java-1.8.0-openjdk-headless wget bash python-pip python-devel sudo netcat gcc gcc-c++ \
+    && yum install -y java-1.8.0-openjdk-headless wget bash python-pip python-devel sudo netcat which gcc gcc-c++ \
     && mkdir -pv /opt \
     && cd /opt \
+    # install zookeeper shell

Review comment:
       @ravisharda maybe this is the only block that is missing in your patch




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] eolivelli commented on issue #2320: Test additional fix for #2219

Posted by GitBox <gi...@apache.org>.
eolivelli commented on issue #2320:
URL: https://github.com/apache/bookkeeper/pull/2320#issuecomment-618438314


   @ravisharda I am trying to use an absolute path for zk-shell
   Let's see if github actions is happy now


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] eolivelli commented on pull request #2320: Test additional fix for #2219

Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #2320:
URL: https://github.com/apache/bookkeeper/pull/2320#issuecomment-622551694


   With this branch I am able to run all of the integration tests locally
   @ravisharda 
   I am going to try to clean up this patch.
   I feel that the problem is about the fact that the init script is executed inside the bookkeeper-current image and so we have to install zk-shell in that Dockerfile as well.
   
   I will try to remove the 'which' tool now 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [bookkeeper] eolivelli commented on a change in pull request #2320: Test additional fix for #2219

Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #2320:
URL: https://github.com/apache/bookkeeper/pull/2320#discussion_r418724313



##########
File path: docker/scripts/init_bookie.sh
##########
@@ -19,65 +19,61 @@
 # * See the License for the specific language governing permissions and
 # * limitations under the License.
 # */
-
 source ${SCRIPTS_DIR}/common.sh
 
+ZKSHELL=$(which zk-shell)

Review comment:
       this is debug
   maybe this is useless
   @ravisharda 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org