Merge pull request #577 from GoogleCloudPlatform/minherz/integrate_ra…

…tings

Implement tech debt and fix #576

.github/workflows/e2e-latest.yml

@@ -104,7 +104,7 @@ jobs:
         # run tests
         python3 tests/monitoring_integration_test.py ${{ env.PROJECT_ID }}
     - name: Run Rating Service Test
-      timeout-minutes: 5
+      timeout-minutes: 30
       run: |
         set -x
         # install dependencies

terraform/ratingservice/configure_rating_db.sh

@@ -31,8 +31,19 @@ echo "Launching Cloud SQL Proxy in background..."
 cloud_sql_proxy -instances=$DB_HOST=tcp:5432 -verbose=false &>/dev/null &
 cloud_proxy_pid=$!
 
-# wait until proxy establishes connection
-sleep 5
+# wait until proxy establishes connection or 5 retries
+tries=0
+while [[ "${tries}" -lt 5 ]]; do
+    echo "Waiting for establishing connection to db..."
+    status_str=$(PGPASSWORD=$DB_PASSWORD psql "host=127.0.0.1 sslmode=disable dbname=$DB_NAME user=$DB_USERNAME" -q -c '\conninfo' 2>/dev/null)
+    if [[ -n "$status_str" ]]; then
+        echo "Connection established"
+        break
+    else
+        sleep 2
+        tries=$((tries + 1))
+    fi
+done
 
 #
 # configure db schema and populate rating entities

terraform/ratingservice/main.tf

@@ -41,8 +41,10 @@ resource "google_project_service" "cloudscheduler" {
   disable_dependent_services = true
 }
 
-resource "random_id" "suffix" {
-  byte_length = 6
+resource "random_string" "suffix_len_4" {
+  upper   = false
+  special = false
+  length  = 4
 }
 
 # provision CloudSQL
@@ -52,7 +54,7 @@ resource "random_password" "db_password" {
 }
 
 resource "google_sql_database_instance" "rating_service" {
-  name                = "ratingservice-sql-instance-${random_id.suffix.hex}"
+  name                = "ratingservice-sql-instance-${random_string.suffix_len_4.result}"
   database_version    = "POSTGRES_12"
   deletion_protection = false
 
@@ -91,7 +93,8 @@ locals {
 }
 
 resource "google_storage_bucket" "it" {
-  name                        = "${var.gcp_project_id}-ratingservice-deployables-${random_id.suffix.hex}"
+  # max name length is 63 char = 30 chars for project id + '-ratingservice-' + 4 char suffix
+  name                        = "${var.gcp_project_id}-ratingservice-${random_string.suffix_len_4.result}"
   uniform_bucket_level_access = true
 }
 

terraform/ratingservice/setup_app_engine.sh

@@ -25,22 +25,16 @@
 # NOTE: Json has string values because Terraform data "external" does not know to parse non-string values
 
 project_id=$1
-ae_app_exist=false
-service_exist=false
 
-print_json_result() {
-  if [ $ae_app_exist == true ]; then
-    app_hostname=$(gcloud app describe --format=json --project=$project_id | jq .defaultHostname)
-    echo "{ \"application_exist\": \"$ae_app_exist\", \"application_domain\": $app_hostname, \"default_svc_exist\": \"$service_exist\" }" | jq .
-  else
-    echo "{ \"application_exist\": \"$ae_app_exist\", \"default_svc_exist\": \"$service_exist\" }" | jq .
-  fi
-}
-
-service_list=$(gcloud app services list --project=$project_id --filter="SERVICE:default" --format=json 2>/dev/null)
-[ $? -eq 0 ] && ae_app_exist=true
-[[ $(echo $service_list | jq length) = 1 ]] && service_exist=true
-print_json_result
+ae_domain=$(gcloud app describe --project=$project_id --format="value(defaultHostname)" 2>/dev/null)
+service_list=$(gcloud app services list --project=$project_id --filter="SERVICE=default" --format="value(SERVICE)" 2>/dev/null)
 
+if [[ -n "$ae_domain" ]]; then
+  default_service=false
+  [[ -n "$service_list" ]] && default_service=true
+  echo "{ \"application_exist\": \"true\", \"default_svc_exist\": \"$default_service\", \"application_domain\": \"$ae_domain\" }" | jq .
+else
+  echo "{ \"application_exist\": \"false\", \"default_svc_exist\": \"false\" }" | jq .
+fi
 
 

tests/ratingservice/main_test.py

@@ -18,41 +18,51 @@
 import sys
 import unittest
 import requests
+from requests.adapters import HTTPAdapter
 import json
-import math
+import decimal
 
 
 service_url = ''
 products = []
 
 
+def composeUrl(resource, eid=""):
+    if not eid:
+        return "{0}/{1}".format(service_url, resource)
+    else:
+        return "{0}/{1}/{2}".format(service_url, resource, eid)
+
+
 class TestEndpoints(unittest.TestCase):
 
-    def composeUrl(self, resource, eid=""):
-        if not eid:
-            return "{0}/{1}".format(service_url, resource)
-        else:
-            return "{0}/{1}/{2}".format(service_url, resource, eid)
+    def setUp(self):
+        self.session = requests.Session()
+        adapter = HTTPAdapter(max_retries=3)
+        self.session.mount('https://', adapter)
+
+    def tearDown(self):
+        self.session.close()
 
     def testGetRating(self):
         """ test getting ratings for all shop products """
         for product in products:
-            url = self.composeUrl("rating", product)
-            res = requests.get(url)
+            url = composeUrl("rating", product)
+            res = self.session.get(url, timeout=1.0)
             self.assertEqual(res.status_code, 200)
 
     def testGetRatingNotExist(self):
         """ test getting rating for non-exist product """
-        url = self.composeUrl("rating", "random")
-        res = requests.get(url)
+        url = composeUrl("rating", "random")
+        res = self.session.get(url, timeout=1.0)
         self.assertEqual(res.status_code, 404)
 
     def testPostNewRating(self):
         """ test posting new rating to a product """
         # use products[0] in "post new vote" test
         test_product_id = products[0]
-        url = self.composeUrl("rating")
-        res = requests.post(url, json={
+        url = composeUrl("rating")
+        res = self.session.post(url, timeout=1.0, json={
             'rating': 5,
             'id': test_product_id
         })
@@ -63,35 +73,36 @@ def testNewRatingCalculation(self):
         # use products[1] to avoid counting new vote from testRate() test
         test_product_id = products[1]
         new_rating_vote = 5
-        url_get = self.composeUrl("rating", test_product_id)
-        url_post = self.composeUrl("rating")
-        url_put = self.composeUrl("recollect")
+        url_get = composeUrl("rating", test_product_id)
+        url_post = composeUrl("rating")
+        url_put = composeUrl("recollect")
 
         # get current rating / post new vote / recollect / get updated rating
-        result1 = requests.get(url_get)
+        result1 = self.session.get(url_get, timeout=1.0)
         self.assertEqual(result1.status_code, 200)
-        result2 = requests.post(url_post, json={
+        result2 = self.session.post(url_post, timeout=1.0, json={
             'rating': new_rating_vote,
             'id': test_product_id
         })
         self.assertEqual(result2.status_code, 200)
-        result2 = requests.put(url_put)
+        result2 = self.session.put(url_put, timeout=1.0)
         self.assertEqual(result2.status_code, 200)
-        result2 = requests.get(url_get)
+        result2 = self.session.get(url_get, timeout=1.0)
         self.assertEqual(result2.status_code, 200)
 
-        result1_data = result1.json()
-        result2_data = result2.json()
-        self.assertEqual(result2_data['votes'],
-                         result1_data['votes'] + 1)
-
-        # new rating should be: old_rating+((5.0-old_rating)/(old_num_of_votes+1))
-        new_rating = result1_data['rating'] + \
-            ((new_rating_vote - result1_data['rating']) /
-             (result1_data['votes'] + 1))
-        # compare new rating vs. expected which was rounded to 4 digits after floating point
-        self.assertEqual(result2_data['rating'],
-                         math.ceil(new_rating*10000)/10000)
+        data = result1.json()
+        prev_vote = data['votes']
+        prev_rating = decimal.Decimal(data['rating'])
+        data = result2.json()
+        new_vote = data['votes']
+        new_rating = decimal.Decimal(data['rating'])
+        self.assertEqual(new_vote, prev_vote + 1)
+        expected_rating = prev_rating + \
+            ((new_rating_vote-prev_rating)/(prev_vote+1))
+        # compare expected result rounded to 4 decimal places
+        QUANTIZE_VALUE = decimal.Decimal("0.0001")
+        self.assertEqual(new_rating.quantize(QUANTIZE_VALUE),
+                         expected_rating.quantize(QUANTIZE_VALUE))
 
 
 def getServiceUrl():
@@ -120,4 +131,4 @@ def getProducts():
 if __name__ == '__main__':
     service_url = getServiceUrl()
     products = getProducts()
-    unittest.main(argv=['first-arg-is-ignored'], verbosity=2)
+    unittest.main(argv=['first-arg-is-ignored'])