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

Docker Compose setup is broken on master #652

Closed
ches opened this issue Apr 26, 2020 · 5 comments · Fixed by #668
Closed

Docker Compose setup is broken on master #652

ches opened this issue Apr 26, 2020 · 5 comments · Fixed by #668
Labels

Comments

@ches
Copy link
Member

ches commented Apr 26, 2020

Expected Behavior

Following https://docs.feast.dev/installation/docker-compose works successfully.

Current Behavior

Spring configurations fail to wire, e.g.

online-serving_1  | 2020-04-26 09:01:16.607  WARN 665e326fd6d8 --- [           main] ConfigServletWebServerApplicationContext : Exception encountered during context initialization - cancelling refresh attempt: org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'storeConfiguration' defined in URL [jar:file:/opt/feast/feast-serving.jar!/BOOT-INF/classes!/feast/serving/configuration/StoreConfiguration.class]: Bean instantiation via constructor failed; nested exception is org.springframework.beans.BeanInstantiationException: Failed to instantiate [feast.serving.configuration.StoreConfiguration$$EnhancerBySpringCGLIB$$eb7624c6]: Constructor threw exception; nested exception is org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'servingRedisConnection' defined in class path resource [feast/serving/configuration/redis/ServingStoreRedisConfig.class]: Bean instantiation via factory method failed; nested exception is org.springframework.beans.BeanInstantiationException: Failed to instantiate [io.lettuce.core.api.StatefulRedisConnection]: Factory method 'servingRedisConnection' threw exception; nested exception is org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'servingRedisClient' defined in class path resource [feast/serving/configuration/redis/ServingStoreRedisConfig.class]: Bean instantiation via factory method failed; nested exception is org.springframework.beans.BeanInstantiationException: Failed to instantiate [io.lettuce.core.RedisClient]: Factory method 'servingRedisClient' threw exception; nested exception is org.springframework.beans.factory.UnsatisfiedDependencyException: Error creating bean with name 'servingStoreRedisConf' defined in class path resource [feast/serving/configuration/redis/ServingStoreRedisConfig.class]: Unsatisfied dependency expressed through method 'servingStoreRedisConf' parameter 0; nested exception is org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'specService' defined in class path resource [feast/serving/configuration/SpecServiceConfig.class]: Bean instantiation via factory method failed; nested exception is org.springframework.beans.BeanInstantiationException: Failed to instantiate [feast.serving.specs.CachedSpecService]: Factory method 'specService' threw exception; nested exception is java.lang.RuntimeException: Unable to update store configuration

Steps to reproduce

Check out Feast master, and:

$ docker-compose -p feast-fullstack --env-file ./infra/docker-compose/.env.sample -f ./infra/docker-compose/docker-compose.yml up

Specifications

  • Version: master
  • Platform: macOS
  • Subsystem: Docker Compose

Possible Solution

This has probably stemmed from the recent configuration refactor merge from #611, like StoreConfiguration being removed and application YAML configs under infra/docker-compose being stale now.

Those config file fragments would need to be updated. It might be a good idea to set image tags for released Feast versions in the .env.sample so that user installation experience following the docs isn't susceptible to mainline development breakages, assuming configs in infra/docker-compose would be frozen until next release time. Alternatively/additionally to the image tags, instruct users to check out a Git tag for the Compose installation.

Dev Workflow Consideration

For development I don't run the full Feast stack Docker Compose, since it isn't set up for that (rebuilding images as you change code, etc.). Rather I run the subset of infra/docker-compose/docker-compose.yml that is only the external service dependencies, and have the running Feast apps connect to the forwarded service ports from my host machine.

I also need the patch below for that to work: there's a transposition typo in the Postgres port, and the Kafka listeners advertise localhost:9094 instead of 9092. Both of those mean that the default application.yml configs using the standard service ports fail to connect out of the box. Perhaps this patch can be included when fixing everything else from the big configuration refactor. I was getting ready to submit this patch when I tested the full stack and found this issue.

diff --git a/infra/docker-compose/docker-compose.yml b/infra/docker-compose/docker-compose.yml
index 38234cff..775687e6 100644
--- a/infra/docker-compose/docker-compose.yml
+++ b/infra/docker-compose/docker-compose.yml
@@ -89,13 +89,12 @@ services:
     environment:
       KAFKA_ZOOKEEPER_CONNECT: zookeeper:2181
       KAFKA_OFFSETS_TOPIC_REPLICATION_FACTOR: 1
-      KAFKA_ADVERTISED_LISTENERS: INSIDE://kafka:9092,OUTSIDE://localhost:9094
-      KAFKA_LISTENERS: INSIDE://:9092,OUTSIDE://:9094
+      KAFKA_ADVERTISED_LISTENERS: INSIDE://kafka:9094,OUTSIDE://localhost:9092
+      KAFKA_LISTENERS: INSIDE://:9094,OUTSIDE://:9092
       KAFKA_LISTENER_SECURITY_PROTOCOL_MAP: INSIDE:PLAINTEXT,OUTSIDE:PLAINTEXT
       KAFKA_INTER_BROKER_LISTENER_NAME: INSIDE
     ports:
       - "9092:9092"
-      - "9094:9094"
 
     depends_on:
       - zookeeper
@@ -110,4 +109,4 @@ services:
     environment:
       POSTGRES_PASSWORD: password
     ports:
-      - "5432:5342"
\ No newline at end of file
+      - "5432:5432"
\ No newline at end of file
@ches ches added the kind/bug label Apr 26, 2020
@ches
Copy link
Member Author

ches commented Apr 26, 2020

An aside for feedback: the jupyter/datascience-notebook is a hefty 5 GB image, many times the size of the rest of the Feast stack. It'd be nice if there's a slimmer alternative.

@khorshuheng
Copy link
Collaborator

@ches I will pick this up.

@woop
Copy link
Member

woop commented Apr 29, 2020

Thanks for raising this @ches.

This has probably stemmed from the recent configuration refactor

Not sure about this. The docker images are set to the latest tag, which means it should be using 0.4.7 which isn't affected by the configuration refactor.

image

Although all of your points above are valid. We should probably have two configurations, one for developers and one for end users to quick start. I believe this was @khorshuheng's intention originally.

@woop
Copy link
Member

woop commented Apr 29, 2020

@ches we can probably solve this through multiple PRs. I tested the following changes #661 locally and it's working for me (I only tested Redis, not BQ).

I much prefer using host based networking so that we don't have to deal with private networks. I also think we should be removing all of the custom infra from our tests and using the docker compose setup for our test suite (unless we need K8s based tests). Probably a logical next step is to break the docker compose files into two. One for development and testing (using master branch) and one that is pinned to a specific version and used as part of the quickstart.

@woop
Copy link
Member

woop commented Apr 29, 2020

Anyway, I have reverted my networking changes in the PR. I kept the typo fixes.

@khorshuheng can you take over from here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants