chore: update cockroach db chart

Spazzy757 commented 1 year ago

Changelog

upgrades CockroachDB Chart (as @nandanrao stated the DB version is set in the values files)
changes script to use upgrade --install for the case where you are making changes to the local deployment

netlify[bot] commented 1 year ago

Deploy request for vlab-research pending review.

Name	Link
Latest commit	e4e323c670e4e4381ba11ebcce9f956092178d94

netlify[bot] commented 1 year ago

Deploy Preview for virtuallab-videos canceled.

Name	Link
Latest commit	e4e323c670e4e4381ba11ebcce9f956092178d94
Latest deploy log	https://app.netlify.com/sites/virtuallab-videos/deploys/63f324ee95af140008f422a3

Spazzy757 commented 1 year ago

Still looking through the code to see how to validate that this upgrade does not have any affect

Spazzy757 commented 1 year ago

From what I can tell all the migrations seem to run without a problem, all the applications seem to start up, however when running the integration tests I get the following errors:

  5 failing

  1) Test Bot flow Survey Integration Testing
       Basic Functionality
         Test chat flow with validation failures:
...
       {
      -  "metadata": "{\"isRepeat\":true,\"validate\":{\"country\":\"IND\",\"mobile\":true},\"ref\":\"5782ed92-3be6-42ce-9621-ca7cba9e5aba\"}"
      +  "metadata": "{\"validate\":{\"country\":\"IND\",\"mobile\":true},\"ref\":\"5782ed92-3be6-42ce-9621-ca7cba9e5aba\"}"
         "quick_replies": [
           {
             "content_type": "user_phone_number"
           }

...

  2) Test Bot flow Survey Integration Testing
       Basic Functionality
         Test chat flow with custom validation error messages:
...
       {
      -  "metadata": "{\"isRepeat\":true,\"ref\":\"9d40efc7-a522-4380-9b5f-101dfcb6c24c\"}"
      +  "metadata": "{\"ref\":\"9d40efc7-a522-4380-9b5f-101dfcb6c24c\"}"
         "text": "Hey - enter a number"
       }
...

  3) Test Bot flow Survey Integration Testing
       Basic Functionality
         Test chat flow on forms with translated responses:
     AssertionError: expected [ null, null ] to include 'LOL'
...

  4) Test Bot flow Survey Integration Testing
       Timeouts
         Sends timeout message response when interrupted in a timeout, then waits:
...

       {
      -  "metadata": "{\"isRepeat\":true,\"type\":\"wait\",\"responseMessage\":\"Please wait!\",\"wait\":{\"type\":\"timeout\",\"value\":\"1 minute\"},\"ref\":\"bd2b2376-d722-4b51-8e1e-c2000ce6ec55\"}"
      +  "metadata": "{\"type\":\"wait\",\"responseMessage\":\"Please wait!\",\"wait\":{\"type\":\"timeout\",\"value\":\"1 minute\"},\"ref\":\"bd2b2376-d722-4b51-8e1e-c2000ce6ec55\"}"
         "text": "Hey, I'll write you again in a bit."
       }
    ...

  5) Test Bot flow Survey Integration Testing
       Timeouts
         Sends follow ups when the user does not respond:
...

       {
      -  "metadata": "{\"isRepeat\":true,\"ref\":\"6ceebf31-4cac-4de8-b760-8bb1f162f145\"}"
      +  "metadata": "{\"ref\":\"6ceebf31-4cac-4de8-b760-8bb1f162f145\"}"
         "quick_replies": [
           {
             "content_type": "text"
             "payload": "{\"value\":\"tired\",\"ref\":\"6ceebf31-4cac-4de8-b760-8bb1f162f145\"}"

it seems like for majority of these the metadata is not setting "isRepeat": true

nandanrao commented 1 year ago

Hrm, the isRepeat": true is related to a changes made recently (actually, on looking it up, I realized that "recently" for me means "within the last 12 months" :D).

Here's where the tests were updated:

https://github.com/vlab-research/fly/commit/d9eca3c2cd917b391f80815d6d73e8b45f92cf4b

But that should have come well after the changes to the app code:

https://github.com/vlab-research/fly/commit/3bee7067e83e1aee206e4b78d75416f1c8f2acd4

This is a bit of a mystery. Unfortunately, I couldn't seem to spin up the dev cluster locally so will need to dig into this when I have a minute.

That being said - help me understand the cockroach changes. Because it looks like you're (A) updating the client and then (B) updating the Chart but I don't see the version that the server will be running anywhere. That lives in the devops/values files, where you will see the server version is 20.1.4 already.

Spazzy757 commented 1 year ago

Ah, this was a misunderstanding on my part (It was the assumption that you were matching the chart and app versions for cockroachDB). Although this does fix the error I was running into around PodDisruptionBudgets, and it shouldn't have any adverse effects on the tests :thinking: I'll update the description. I have a bit of a busy day today but I will circle back around to this tomorrow

nandanrao commented 1 year ago

That makes sense, I like it! Getting the right pod disruption budget will help!

nandanrao commented 1 year ago

Hey sorry - the problem with the isRepeat true is essentially being caused by our lack of CI right now (which we haven't had for over a year, since Docker cut it out of the free plan :D).

What happened was that changes were made to the test file (facebot/testrunner/test.js), however, I only ran it locally (dev.sh in facebot/testrunner) and never built the new testrunner docker image and pushed that / updated that.

I just pushed some changes, so if you rebase now you should be able to run this just fine with kind?

Spazzy757 commented 1 year ago

Yup, all tests pass with the changes, fantastic!

What happened was that changes were made to the test file (facebot/testrunner/test.js), however, I only ran it locally (dev.sh in facebot/testrunner) and never built the new testrunner docker image and pushed that / updated that.

Could we just move this over to CircveCI and/or Github Actions (They are mostly free on Open Source Projects)?

nandanrao commented 1 year ago

Yeah we definitely can - we had the integration tests set up on CircleCI before, it just needs some maintenance, that's all!

vlab-research / fly