confluentinc / schema-registry-images

Docker Images for Schema Registry
Apache License 2.0
2 stars 21 forks source link

Referencing KAFKA_REST environment variables in the schema-registry image #18

Closed russau closed 4 years ago

russau commented 4 years ago

This looks like a bug: the ensure script in the schema-registry image is referring to KAFKA_REST_ environment variables. Should this be SCHEMA_REGISTRY_CUB_ environment variables?

https://github.com/confluentinc/schema-registry-images/blob/0dc71db9719cf84c707771141010dcf471149b50/schema-registry/include/etc/confluent/docker/ensure#L42-L46

Attached zip will repro the issue. I've substituted an ensure script with some debugging statements. If you unzip it docker-compose up, then check the logs for the schema registry container you'll see the debugging output:

!!!!! DEBUGGING !!!!!!!
Waiting for 3 brokers
Waiting for 123 seconds timeout

ensure is using the environment variables KAFKA_REST_CUB_KAFKA_MIN_BROKERS and KAFKA_REST_CUB_KAFKA_TIMEOUT where it should be the SCHEMA_REGISTRY_CUB_ environment variables.

repro.zip

russau commented 4 years ago

Was this fix planned for the 5.5 containers. I'm still seeing the issue:

$ docker run -it --rm confluentinc/cp-schema-registry:5.5.0-1-ubi8 cat /etc/confluent/docker/ensure
#!/usr/bin/env bash
#
# Copyright 2016 Confluent Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

. /etc/confluent/docker/bash-config

if [[ -n "${SCHEMA_REGISTRY_KAFKASTORE_CONNECTION_URL-}" ]]
then
    echo "===> Check if Zookeeper is healthy ..."
    cub zk-ready "$SCHEMA_REGISTRY_KAFKASTORE_CONNECTION_URL" "${SCHEMA_REGISTRY_CUB_ZK_TIMEOUT:-40}"
fi

echo "===> Check if Kafka is healthy ..."

if [[ -n "${SCHEMA_REGISTRY_KAFKASTORE_SECURITY_PROTOCOL-}" ]] && [[ $SCHEMA_REGISTRY_KAFKASTORE_SECURITY_PROTOCOL != "PLAINTEXT" ]]
then
    cub kafka-ready \
        "${SCHEMA_REGISTRY_CUB_KAFKA_MIN_BROKERS:-1}" \
        "${SCHEMA_REGISTRY_CUB_KAFKA_TIMEOUT:-40}" \
        -b "${SCHEMA_REGISTRY_KAFKASTORE_BOOTSTRAP_SERVERS}" \
        --config /etc/"${COMPONENT}"/admin.properties
else
    if [[ -n "${SCHEMA_REGISTRY_KAFKASTORE_CONNECTION_URL-}" ]]
    then
        cub kafka-ready \
            "${SCHEMA_REGISTRY_CUB_KAFKA_MIN_BROKERS:-1}" \
            "${SCHEMA_REGISTRY_CUB_KAFKA_TIMEOUT:-40}" \
            -z "$SCHEMA_REGISTRY_KAFKASTORE_CONNECTION_URL"
    elif [[ -n "${SCHEMA_REGISTRY_KAFKASTORE_BOOTSTRAP_SERVERS-}" ]]
    then
        cub kafka-ready \
            "${KAFKA_REST_CUB_KAFKA_MIN_BROKERS:-1}" \
            "${KAFKA_REST_CUB_KAFKA_TIMEOUT:-40}" \
            -b "${SCHEMA_REGISTRY_KAFKASTORE_BOOTSTRAP_SERVERS}"
    fi
fi
rayokota commented 4 years ago

@russau, I cherry-picked the fix to 5.4.x and 5.5.x

russau commented 4 years ago

@rayokota thanks for taking a look. Does this mean we'll push new 5.5 images to dockerhub? Or the fix will be in future patch release images?

rayokota commented 4 years ago

It'll be in future patch release images