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

[kots]: add database to preflight checks #9759

Merged
merged 1 commit into from
Jun 1, 2022
Merged

Conversation

mrsimonemms
Copy link
Contributor

@mrsimonemms mrsimonemms commented May 4, 2022

Description

Add a database check to the KOTS preflight checks. This validates that the given configuration is able to make a connection to a MySQL database called gitpod and that the version is 5.7.x - this version check is considered a warning because it's possible that we get anomalies with the versions

Failed

image

Pass

image

Related Issue(s)

Fixes #9931

How to test

Install with KOTS - the version to use is sje-kots-config-check.11 in sje/kots-config-check channel

Now change the config values for database in KOTS:

  • in-cluster should always pass for both checks
  • external should pass with correct credentials (check that DB's firewall allows connections - can make public with 0.0.0.0/0)
  • cloud SQL should pass with correct credentials (ensure firewall is closed)

You can create a support bundle and look in the file explorer for database/database.log to see the database that's output

Release Notes

[kots]: add database to preflight checks

Documentation

@mrsimonemms mrsimonemms force-pushed the sje/kots-config-check branch 2 times, most recently from d96f0d5 to 9b8a2d2 Compare May 4, 2022 13:55
@mrsimonemms mrsimonemms changed the title [kots]: add database to preflight checks WIP: [kots]: add database to preflight checks May 4, 2022
@mrsimonemms mrsimonemms force-pushed the sje/kots-config-check branch 4 times, most recently from 17feeab to 4e94f59 Compare May 11, 2022 09:13
@roboquat roboquat added size/L and removed size/M labels May 11, 2022
@mrsimonemms mrsimonemms force-pushed the sje/kots-config-check branch 3 times, most recently from aaaebe5 to b79a580 Compare May 11, 2022 09:38
@mrsimonemms
Copy link
Contributor Author

mrsimonemms commented May 11, 2022

/werft run no-preview publish-to-kots

👍 started the job as gitpod-build-sje-kots-config-check.10
(with .werft/ from main)

@mrsimonemms
Copy link
Contributor Author

mrsimonemms commented May 11, 2022

/werft run no-preview publish-to-kots

👍 started the job as gitpod-build-sje-kots-config-check.11
(with .werft/ from main)

@mrsimonemms mrsimonemms changed the title WIP: [kots]: add database to preflight checks [kots]: add database to preflight checks May 11, 2022
@mrsimonemms mrsimonemms marked this pull request as ready for review May 11, 2022 10:21
@mrsimonemms mrsimonemms requested a review from a team May 11, 2022 10:21
@github-actions github-actions bot added the team: delivery Issue belongs to the self-hosted team label May 11, 2022
@mrsimonemms mrsimonemms force-pushed the sje/kots-config-check branch from b79a580 to 9271a07 Compare May 11, 2022 13:29
Copy link
Contributor

@corneliusludmann corneliusludmann left a comment

Choose a reason for hiding this comment

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

Thanks for adding this check. That is very useful! 🚀

Added a few comments. The question, of whether this will work for air-gap is the most important one. I would expect that is works. However, we need to make sure before we release it.

The other comments can be handled in follow-up issues and are no blockers, in my opinion.

@@ -70,6 +70,7 @@ packages:
- components/ws-manager:docker
- components/ws-proxy:docker
- components/ide-proxy:docker
- components/kots-config-check/database:docker
Copy link
Contributor

Choose a reason for hiding this comment

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

Small optimization suggestion:

For deploying a preview env, we don't need to build this image. We need this only when we deploy a KOTS release. Building this only when the publish-to-kots werft flag is set would help us to not extend the time to wait for a preview env for every dev.

Note: We can handle this in a follow up PR and create an issue and merge the PR as is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that would be a good optimisation. Is there an example I can use to follow as I didn't know this was possible?

@@ -7,6 +7,19 @@ metadata:
name: gitpod
spec:
collectors:
- run:
collectorName: database
image: eu.gcr.io/gitpod-core-dev/build/kots-config-check/database:sje-kots-config-check.9
Copy link
Contributor

Choose a reason for hiding this comment

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

I think, in .werft/jobs/build/build-and-publish.ts we need to set the version tag in this file, right?

We could get the version with something like this:

const kotsConfigCheckDatabaseTag = exec(`docker run --rm eu.gcr.io/gitpod-core-dev/build/versions:${jobConfig.version} cat /versions.yaml | yq r - 'components.kots-config-check.database.version'`, { silent: true })
            .stdout.trim();

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 don't think so. My understanding is that this will only be built by Werft if it detects a change - as this is very unlikely to change, this won't be built on each push. I think this is more akin to our .gitpod.yml image

@@ -7,6 +7,19 @@ metadata:
name: gitpod
spec:
collectors:
- run:
collectorName: database
image: eu.gcr.io/gitpod-core-dev/build/kots-config-check/database:sje-kots-config-check.9
Copy link
Contributor

Choose a reason for hiding this comment

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

In .werft/jobs/build/build-and-publish.ts we need to set the version tag in this file. (same as above)

install/kots/manifests/kots-preflight.yaml Show resolved Hide resolved
--host="${DB_HOST}" \
--port="${DB_PORT}" \
--execute="SELECT VERSION();" \
--silent \
Copy link
Contributor

Choose a reason for hiding this comment

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

Random thought: Do we still get enough log data with the silent flag for the support bundle or should we remove this flag?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. The --silent flag basically removes all the connection information. At this point, we're only interested in the version so we can assign it to the version_query variable. It is therefore correct that we have the --silent flag.

Also, there's actually no way of debugging a preflight check so we are limited to just putting the data in a known format so we can perform regex queries on it.

This checks the connection and the version is correct, based upon the
configuration given.
@mrsimonemms mrsimonemms force-pushed the sje/kots-config-check branch from 9271a07 to 5f69faf Compare May 30, 2022 15:15
@mrsimonemms
Copy link
Contributor Author

mrsimonemms commented May 31, 2022

/werft run

👍 started the job as gitpod-build-sje-kots-config-check.14
(with .werft/ from main)

@mrsimonemms mrsimonemms marked this pull request as ready for review May 31, 2022 07:36
Copy link
Contributor

@corneliusludmann corneliusludmann left a comment

Choose a reason for hiding this comment

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

Thanks for the clarifications. Ship it! 🚢 😆

@roboquat roboquat merged commit 1166474 into main Jun 1, 2022
@roboquat roboquat deleted the sje/kots-config-check branch June 1, 2022 12:16
@mrsimonemms
Copy link
Contributor Author

mrsimonemms commented Jun 1, 2022

🚢 🚢 🚢 🚢 🚢 🚢 🚢 🚢 🚢 🚢 🚢 🚢

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note size/L team: delivery Issue belongs to the self-hosted team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

KOTS database preflight checks
3 participants