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

fix(plausible): migrate to new common chart and upgrade to v2 #11186

Merged
merged 17 commits into from
Aug 12, 2023

Conversation

schnerring
Copy link
Contributor

@schnerring schnerring commented Aug 4, 2023

Description

TODO:

  • upgrade questions.yaml to new common chart
  • migrate common.yaml / _env.tpl to new common chart format

⚒️ Fixes #

⚙️ Type of change

  • ⚙️ Feature/App addition
  • 🪛 Bugfix
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 🔃 Refactor of current code

🧪 How Has This Been Tested?

📃 Notes:

✔️ Checklist:

  • ⚖️ My code follows the style guidelines of this project
  • 👀 I have performed a self-review of my own code
  • #️⃣ I have commented my code, particularly in hard-to-understand areas
  • 📄 I have made corresponding changes to the documentation
  • ⚠️ My changes generate no new warnings
  • 🧪 I have added tests to this description that prove my fix is effective or that my feature works
  • ⬆️ I increased versions for any altered app according to semantic versioning

➕ App addition

If this PR is an app addition please make sure you have done the following.

  • 🪞 I have opened a PR on truecharts/containers adding the container to TrueCharts mirror repo.
  • 🖼️ I have added an icon in the Chart's root directory called icon.png

Please don't blindly check all the boxes. Read them and only check those that apply.
Those checkboxes are there for the reviewer to see what is this all about and
the status of this PR with a quick glance.

@schnerring schnerring force-pushed the upgrade-plausible branch 4 times, most recently from f300bdc to 32bbef2 Compare August 4, 2023 15:17
@schnerring
Copy link
Contributor Author

schnerring commented Aug 4, 2023

Any idea what to do about this linting error?

Error: execution error at (plausible/templates/common.yaml:2:4): Operator [cloudnative-pg] has to be installed first

@stavros-k
Copy link
Collaborator

Any idea what to do about this linting error?

Error: execution error at (plausible/templates/common.yaml:2:4): Operator [cloudnative-pg] has to be installed first

Commented out the helm template from the linting stage, as it would require operators to be installed there as well.
Should be passing now

@stavros-k
Copy link
Collaborator

For catalog test failing, you can "generate" questions and see for errors
From the root of the project

./tools/gen-cat.sh charts/incubator/plausible

Output will go to a new dir on the root of the project called catalog

@schnerring
Copy link
Contributor Author

./tools/gen-cat.sh charts/incubator/plausible

That command returns the following error when running it:

Error: no cached repository for helm-manager-abaa37bfaf705b7726f614114b77dd23196b42f5087901e57467be7fa3fead01 found. (try 'helm repo update'): open /home/***/.cache/helm/repository/helm-manager-abaa37bfaf705b7726f614114b77dd23196b42f5087901e57467be7fa3fead01-index.yaml: no such file or directory

Any idea what I'm missing?

@stavros-k
Copy link
Collaborator

./tools/gen-cat.sh charts/incubator/plausible

That command returns the following error when running it:

Error: no cached repository for helm-manager-abaa37bfaf705b7726f614114b77dd23196b42f5087901e57467be7fa3fead01 found. (try 'helm repo update'): open /home/***/.cache/helm/repository/helm-manager-abaa37bfaf705b7726f614114b77dd23196b42f5087901e57467be7fa3fead01-index.yaml: no such file or directory

Any idea what I'm missing?

Mine seem to work, maybe you need to add the repositories

❯ ./tools/gen-cat.sh charts/incubator/plausible 
No owner configured, defaulting to truecharts
No repo configured, defaulting to charts
env #CR_TOKEN not found, defaulting to production=false
"truecharts" already exists with the same configuration, skipping
"truecharts-library" already exists with the same configuration, skipping
Hang tight while we grab the latest from your chart repositories...
...Successfully got an update from the "truecharts-library" chart repository
...Successfully got an update from the "truecharts-deps" chart repository
...Successfully got an update from the "truecharts" chart repository
...Successfully got an update from the "bitnami" chart repository
...Successfully got an update from the "prometheus" chart repository
Update Complete. ⎈Happy Helming!⎈
Start processing charts/incubator/plausible ...
Saving 3 charts
Downloading common from repo https://library-charts.truecharts.org
Downloading postgresql from repo https://deps.truecharts.org/
Downloading clickhouse from repo https://deps.truecharts.org/
Deleting outdated charts
Cleaning SCALE catalog for Chart: plausible
Copying Chart to Catalog: plausible
Applying SCALE patches for Chart: plausible
Making copy of charts/incubator/plausible/questions.yaml to catalog/incubator/plausible/4.0.25/questions.yaml
Including standardised questions.yaml includes for: plausible
Removing old versions for: plausible
/home/skois/projects/truecharts/apps
Done processing charts/incubator/plausible ...
Starting post-processing

@schnerring
Copy link
Contributor Author

Mine seem to work, maybe you need to add the repositories

tools/get-cat.sh was missing the truecharts-deps repo, I added it to the prep_helm function of the script.

Regarding questions.yaml, I didn't really know what I was doing / looking for, so I just did a side-by-side comparison with the up-to-date paperless-ngx questions.yaml and changed the plausible questions.yaml accordingly.

Now the "install charts" CI step fails. It looks like ClickHouse is the issue. Here's the event log of the plausible-luh9hueop1-clickhouse-5bd7db77f8-qnfql pod:

Events:
  Type     Reason     Age                    From               Message
  ----     ------     ----                   ----               -------
  Normal   Scheduled  4m4s                   default-scheduler  Successfully assigned plausible-luh9hueop1/plausible-luh9hueop1-clickhouse-5bd7db77f8-qnfql to k3d-k3s-default-server-0
  Normal   Pulling    4m3s                   kubelet            Pulling image "tccr.io/truecharts/clickhouse:v23.7.1.2470@sha256:fbec51f0c6b09733fe4601bba2584dc750f0a8a5077702b86163744bd6f4573f"
  Normal   Pulled     3m46s                  kubelet            Successfully pulled image "tccr.io/truecharts/clickhouse:v23.7.1.2470@sha256:fbec51f0c6b09733fe4601bba2584dc750f0a8a5077702b86163744bd6f4573f" in 16.389378822s
  Normal   Pulled     102s (x11 over 3m46s)  kubelet            Container image "tccr.io/truecharts/clickhouse:v23.7.1.2470@sha256:fbec51f0c6b09733fe4601bba2584dc750f0a8a5077702b86163744bd6f4573f" already present on machine
  Warning  Failed     102s (x12 over 3m46s)  kubelet            Error: secret "clickhousecreds" not found

I guess this is due the common chart refactorings to the ClickHouse chart / library-chart? Gotta do some more debugging..........

@schnerring
Copy link
Contributor Author

Things are looking good! However, CNPG fails to start up in the Install charts Part 1 tests. Any idea why? I don't see anything in the logs.

 Events:
  Type     Reason     Age                    From               Message
  ----     ------     ----                   ----               -------
  Normal   Scheduled  3m36s                  default-scheduler  Successfully assigned plausible-vu2tnxzq9l/plausible-vu2tnxzq9l-cnpg-main-1 to k3d-k3s-default-server-0
  Normal   Pulled     3m37s                  kubelet            Container image "tccr.io/truecharts/cloudnative-pg:v1.20.2@sha256:07121d674c1de587044746f92cc3bccb4576f2fc125f6f2f5bcaa582dad3e3ad" already present on machine
  Normal   Created    3m37s                  kubelet            Created container bootstrap-controller
  Normal   Started    3m36s                  kubelet            Started container bootstrap-controller
  Normal   Pulled     3m36s                  kubelet            Container image "ghcr.io/cloudnative-pg/postgresql:15.3" already present on machine
  Normal   Created    3m36s                  kubelet            Created container postgres
  Normal   Started    3m36s                  kubelet            Started container postgres
  Warning  Unhealthy  3m33s (x3 over 3m35s)  kubelet            Readiness probe failed: HTTP probe failed with statuscode: 500

@stavros-k
Copy link
Collaborator

Things are looking good! However, CNPG fails to start up in the Install charts Part 1 tests. Any idea why? I don't see anything in the logs.

 Events:
  Type     Reason     Age                    From               Message
  ----     ------     ----                   ----               -------
  Normal   Scheduled  3m36s                  default-scheduler  Successfully assigned plausible-vu2tnxzq9l/plausible-vu2tnxzq9l-cnpg-main-1 to k3d-k3s-default-server-0
  Normal   Pulled     3m37s                  kubelet            Container image "tccr.io/truecharts/cloudnative-pg:v1.20.2@sha256:07121d674c1de587044746f92cc3bccb4576f2fc125f6f2f5bcaa582dad3e3ad" already present on machine
  Normal   Created    3m37s                  kubelet            Created container bootstrap-controller
  Normal   Started    3m36s                  kubelet            Started container bootstrap-controller
  Normal   Pulled     3m36s                  kubelet            Container image "ghcr.io/cloudnative-pg/postgresql:15.3" already present on machine
  Normal   Created    3m36s                  kubelet            Created container postgres
  Normal   Started    3m36s                  kubelet            Started container postgres
  Warning  Unhealthy  3m33s (x3 over 3m35s)  kubelet            Readiness probe failed: HTTP probe failed with statuscode: 500

I'm not sure that its stays Unhealthy, its just the last event logged. If it stayed unhealthy it would be somelike like x20-x30 over the time.

What I've seen is this

------------------------------------------------------------------------------------------------------------------------
==> Logs of init container plausible-vu2tnxzq9l-c6665c7b6-8rlr8
------------------------------------------------------------------------------------------------------------------------
Loading plausible..
Starting dependencies..
Starting repos..
create Plausible.Repo database if it doesn't exist
create Plausible.IngestRepo database if it doesn't exist
Creation of Db successful!
Loading plausible..
Starting dependencies..
Starting repos..
Running migrations for Elixir.Plausible.Repo
Running migrations for Elixir.Plausible.IngestRepo
Migrations successful!
------------------------------------------------------------------------------------------------------------------------
<== Logs of init container plausible-vu2tnxzq9l-c6665c7b6-8rlr8
------------------------------------------------------------------------------------------------------------------------
------------------------------------------------------------------------------------------------------------------------
==> Logs of init container plausible-vu2tnxzq9l-c6665c7b6-8rlr8
------------------------------------------------------------------------------------------------------------------------
/bin/sh: /bin/bash: not found
------------------------------------------------------------------------------------------------------------------------
<== Logs of init container plausible-vu2tnxzq9l-c6665c7b6-8rlr8
------------------------------------------------------------------------------------------------------------------------

Is there any chance the migration entrypoint.sh trying to use bash?

@stavros-k
Copy link
Collaborator

Signed-off-by: Stavros Kois <47820033+stavros-k@users.noreply.github.com>
@stavros-k stavros-k marked this pull request as ready for review August 12, 2023 09:55
@stavros-k stavros-k requested review from PrivatePuffin and a team as code owners August 12, 2023 09:55
@stavros-k stavros-k enabled auto-merge (squash) August 12, 2023 10:05
@stavros-k stavros-k merged commit 5b1058a into truecharts:master Aug 12, 2023
14 checks passed
@schnerring
Copy link
Contributor Author

@schnerring Found the issue truecharts/library-charts@48b8e12

Nice find, thanks for the help!

@truecharts-admin
Copy link
Collaborator

This PR is locked to prevent necro-posting on closed PRs. Please create a issue or contact staff on discord if you want to further discuss this

@truecharts truecharts locked as resolved and limited conversation to collaborators Aug 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants