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

vm-builder: Turn off --auto-discover-databases #571

Merged
merged 1 commit into from
Oct 20, 2023

Conversation

ololobus
Copy link
Member

It causes exporter connection to every database in the Postgres and it was disabled for pods a while ago, but I forgot that VMs have a different build path.

See this for details:
https://github.com/neondatabase/cloud/commit/0a60057f3380e570b540cc823975e07400cd640c

Also remove --exclude-databases it seems to be bogus as well, see:
prometheus-community/postgres_exporter#588

It causes exporter connection to every database in the Postgres and it
was disabled for pods a while ago, but I forgot that VMs have a different
build path.

See this for details:
  neondatabase/cloud@0a60057

Also remove `--exclude-databases` it seems to be bogus as well, see:
  prometheus-community/postgres_exporter#588
@ololobus
Copy link
Member Author

ololobus commented Oct 19, 2023

Hm, looks like one of the tests fails (seems to be related to live migration no, migration passed, but autoscaling test failed)

Finished	Migration finished with success to target pod (example-dvcr5)		
    logger.go:42: 15:48:53 | vm-migration | Deleting namespace: kuttl-test-golden-coral
=== CONT  kuttl
    harness.go:405: run tests finished
    harness.go:513: cleaning up
    harness.go:570: removing temp folder: ""
--- FAIL: kuttl (369.85s)
    --- FAIL: kuttl/harness (0.00s)
        --- FAIL: kuttl/harness/autoscaling (338.96s)
        --- PASS: kuttl/harness/vm-migration (29.97s)
FAIL
make: *** [Makefile:365: e2e] Error 1

https://github.com/neondatabase/autoscaling/actions/runs/6574880494/job/17862265473?pr=571#step:15:218

But I don't get what the error is. @sharnoff can you please help with reading this test result?

@sharnoff
Copy link
Member

Looks like downscale is failing because the vm-monitor is hitting internal errors. Unfortunately we don't have logs for the VMs themselves, but it's unlikely that it's failing because of this PR — each vm-monitor is pulled from neondatabase/neon's current main, so maybe some recent change or something.

I'll dig into it further.

@ololobus
Copy link
Member Author

OK, I'd avoid stacking PR on top of failing tests and it's not urgent, so let me know when it makes sense to re-run / safe to merge

Copy link
Member

@sharnoff sharnoff left a comment

Choose a reason for hiding this comment

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

Problem was fixed with neondatabase/neon#5606, all good to merge.

@ololobus ololobus merged commit cdcf6be into main Oct 20, 2023
8 checks passed
@ololobus ololobus deleted the alexk/turn-off-autodiscover branch October 20, 2023 06:24
@sharnoff
Copy link
Member

Filed #577 to track the long term solution for moving postgres_exporter configuration into the control plane (among other things).

There's more immediate changes that could be made to make that simpler, if we anticipate needing to do this frequently or with urgency.

sharnoff added a commit to neondatabase/neon that referenced this pull request Oct 24, 2023
Only applicable change was neondatabase/autoscaling#571, removing the
postgres_exporter flags `--auto-discover-databases` and
`--exclude-databases=...`
sharnoff added a commit to neondatabase/neon that referenced this pull request Oct 24, 2023
Only applicable change was neondatabase/autoscaling#571, removing the
postgres_exporter flags `--auto-discover-databases` and
`--exclude-databases=...`
bayandin pushed a commit to neondatabase/neon that referenced this pull request Oct 26, 2023
Only applicable change was neondatabase/autoscaling#571, removing the
postgres_exporter flags `--auto-discover-databases` and
`--exclude-databases=...`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants