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

Deploy shopping cart on centralpark2 #40

Merged
merged 13 commits into from
May 21, 2019

Conversation

ignasi35
Copy link
Contributor

@ignasi35 ignasi35 commented May 17, 2019

Fixes #16

oc version && echo "oc CLI installed successfully"
}

installKustomize() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kubectl 1.14+ have kustomize out-of-the-box. We played with him. OC until haven't?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting.
TBH, I added this code here but it's not used and we'll probably remove it. We've found other ways to manage the YAML files without kustomize. This is a left-over from lagom/lagom-scala-openshift-smoketests

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've found other ways to manage the YAML files without kustomize

We too 😄
Now for Lagom 1.5.X we migrated from LBO to HELM charts.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyway, I think oc 3.11 uses kubectl 1.11 :-/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realised over the weekend that kustomize being part of kubectl will require us changing some docs. Thanks for sharing @ihostage ! ❤️

@ignasi35 ignasi35 force-pushed the deploy-shopping-cart-on-centralpark branch 3 times, most recently from abcc33b to dec518c Compare May 17, 2019 21:16
@ignasi35 ignasi35 force-pushed the deploy-shopping-cart-on-centralpark branch from 7d454f9 to b997461 Compare May 18, 2019 12:11
@ignasi35
Copy link
Contributor Author

building the docker image in maven fails on travis:

-------------------------------------------------------
 T E S T S
-------------------------------------------------------
Running com.example.shoppingcart.impl.ShoppingCartEntityTest
Tests run: 10, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.908 sec
Results :
Tests run: 10, Failures: 0, Errors: 0, Skipped: 0
[INFO] 
[INFO] --- maven-jar-plugin:2.4:jar (default-jar) @ shopping-cart ---
[INFO] Building jar: /home/travis/build/lagom/lagom-samples/shopping-cart/shopping-cart-java/shopping-cart/target/shopping-cart-1.0-SNAPSHOT.jar
[INFO] 
[INFO] --- docker-maven-plugin:0.26.1:build (build-docker-image) @ shopping-cart ---
[ERROR] DOCKER> Unable to pull 'adoptopenjdk/openjdk8:latest' from registry 'docker-registry-default.centralpark2.lightbend.com' : unauthorized: authentication required (Internal Server Error: 500) [unauthorized: authentication required (Internal Server Error: 500)]
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary:
[INFO] 

I've managed to reproduce locally by removing all images from the registries I was logged to:

docker images --format='{{println .ID}}'| xargs docker rmi -f  | wc -l 

I think the problem is that fabric8's docker-maven-plugin reads the DOCKER_REGISTRY env var that we use on the scripts. Another cause for the problem could be that centralpark's registry is set to not fetch images it doesn't contain.

@TimMoore
Copy link

This will resolve #16, right?

@ignasi35
Copy link
Contributor Author

This will resolve #16, right?

yes, I've updated the description

@ignasi35
Copy link
Contributor Author

ignasi35 commented May 20, 2019

Me not pressing send to publish the #40 (comment) and also me squashing commits left the history on this PR a bit confusing. Here's the current status:

  • This PR already solves the Unable to pull 'adoptopenjdk/openjdk8:latest' from registry issue. The bug was caused by our scripts declaring the DOCKER_REGISTRY ENV_VAR which causes docker-maven-plugin to behave differently.

At this point, the only issue pending solution is the installation of Kafka/Strimzi on CentralPark2.

then
(
cd $SHOPPING_CART_SOURCES
buildSbt $SHOPPING_CART_SOURCES
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
buildSbt $SHOPPING_CART_SOURCES
sbt clean docker:publishLocal

We can simply inline it here.

then
(
cd $SHOPPING_CART_SOURCES
buildMvn $SHOPPING_CART_SOURCES
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
buildMvn $SHOPPING_CART_SOURCES
mvn package docker:build

}
buildMvn() {
mvn package docker:build
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From line 14 to 19 can be removed if inlined below

if [ "$BUILD_TOOL" == "sbt" ]
then
(
cd $SHOPPING_CART_SOURCES
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can cd outside the if, but inside a ( ) scope block

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with the ( ) scoping rules so I trust your changes here. :-)

@octonato octonato merged commit 55def0b into 1.5.x May 21, 2019
@mergify mergify bot deleted the deploy-shopping-cart-on-centralpark branch May 21, 2019 20:12
dwijnand pushed a commit to dwijnand/lagom-samples that referenced this pull request Aug 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Add an automated deployment test
4 participants