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(migration): add last_heartbet and ready as column of server_instances tables in a separate migration #233

Merged
merged 1 commit into from
Dec 17, 2024

Conversation

machi1990
Copy link
Contributor

fix(migration): add last_heartbet and ready as column of server_instances tables in a separate migration

Addresses https://github.com/openshift-online/maestro/pull/229/files#r1888513628 as the issue is causing existing deployment to fail with errors like

2024-12-17T13:19:34.107Z	ERROR	server/healthcheck_server.go:115	Unable to create maestro instance: ERROR: column "last_heartbeat" of relation "server_instances" does not exist (SQLSTATE 42703)
github.com/openshift-online/maestro/cmd/maestro/server.(*HealthCheckServer).pulse

or

2024-12-17T13:19:54.339Z	ERROR	server/healthcheck_server.go:184	Unable to mark inactive maestro instances ([maestro-maestro-689869f6dd-5jsbs maestro-maestro-689869f6dd-6tgxq]): ERROR: column "ready" of relation "server_instances" does not exist (SQLSTATE 42703)
github.com/openshift-online/maestro/cmd/maestro/server.(*HealthCheckServer).checkInstances

…nces tables in a separate migration

Addresses https://github.com/openshift-online/maestro/pull/229/files#r1888513628 as the issue is causing existing deployment to fail with errors like
```
2024-12-17T13:19:34.107Z	ERROR	server/healthcheck_server.go:115	Unable to create maestro instance: ERROR: column "last_heartbeat" of relation "server_instances" does not exist (SQLSTATE 42703)
github.com/openshift-online/maestro/cmd/maestro/server.(*HealthCheckServer).pulse
```

or

```
2024-12-17T13:19:54.339Z	ERROR	server/healthcheck_server.go:184	Unable to mark inactive maestro instances ([maestro-maestro-689869f6dd-5jsbs maestro-maestro-689869f6dd-6tgxq]): ERROR: column "ready" of relation "server_instances" does not exist (SQLSTATE 42703)
github.com/openshift-online/maestro/cmd/maestro/server.(*HealthCheckServer).checkInstances
```
@machi1990 machi1990 requested a review from clyang82 December 17, 2024 13:42
@clyang82
Copy link
Contributor

Here is the integration error:

=== RUN   TestConsumerGet
I1217 13:48:36.117311   23046 api_server.go:151] Serving without TLS at 8000
{"level":"info","ts":1734443316.1173534,"logger":"fallback","caller":"[email protected]/protocol.go:133","msg":"subscribing to topics: [{sources/maestro/consumers/+/agentevents 1 0 false false}]"}
E1217 13:48:36.120090   23046 healthcheck_server.go:147] Unable to get all maestro instances: pq: relation "server_instances" does not exist
E1217 13:48:36.120119   23046 logger.go:129]   Unable to get all maestro instances: pq: relation "server_instances" does not exist 
E1217 13:48:36.120239   23046 healthcheck_server.go:119] Unable to get maestro instance: pq: relation "server_instances" does not exist
E1217 13:48:36.120451   23046 panic.go:261] "Observed a panic" panic="runtime error: invalid memory address or nil pointer dereference" panicGoValue="\"invalid memory address or nil pointer dereference\"" stacktrace=<
	goroutine 50 [running]:
	k8s.io/apimachinery/pkg/util/runtime.logPanic({0x2e9b390, 0x47b3de0}, {0x2613e60, 0x46d95f0})
		/root/go/pkg/mod/k8s.io/[email protected]/pkg/util/runtime/runtime.go:107 +0xbc
	k8s.io/apimachinery/pkg/util/runtime.handleCrash({0x2e9b390, 0x47b3de0}, {0x2613e60, 0x46d95f0}, {0x47b3de0, 0x0, 0x100000002458e80?})
		/root/go/pkg/mod/k8s.io/[email protected]/pkg/util/runtime/runtime.go:82 +0x5e
	k8s.io/apimachinery/pkg/util/runtime.HandleCrash({0x0, 0x0, 0x24?})
		/root/go/pkg/mod/k8s.io/[email protected]/pkg/util/runtime/runtime.go:59 +0x108
	panic({0x2613e60?, 0x46d95f0?})
		/usr/local/go/src/runtime/panic.go:770 +0x132
	github.com/openshift-online/maestro/cmd/maestro/server.(*HealthCheckServer).pulse(0xc0007aafc0, {0x2e9b438, 0xc0007b2230})
		/maestro/cmd/maestro/server/healthcheck_server.go:121 +0x6a0
	k8s.io/apimachinery/pkg/util/wait.JitterUntilWithContext.func1()
		/root/go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/backoff.go:259 +0x1f
	k8s.io/apimachinery/pkg/util/wait.BackoffUntil.func1(0x30?)
		/root/go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/backoff.go:226 +0x33
	k8s.io/apimachinery/pkg/util/wait.BackoffUntil(0xc000925f70, {0x2e77220, 0xc000558000}, 0x1, 0xc0001142a0)
		/root/go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/backoff.go:227 +0xaf
	k8s.io/apimachinery/pkg/util/wait.JitterUntil(0xc000469770, 0x3b9aca00, 0x0, 0x1, 0xc0001142a0)
		/root/go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/backoff.go:204 +0x7f
	k8s.io/apimachinery/pkg/util/wait.JitterUntilWithContext({0x2e9b438, 0xc0007b2230}, 0xc0002ca010, 0x3b9aca00, 0x0, 0x1)
		/root/go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/backoff.go:259 +0x93
	k8s.io/apimachinery/pkg/util/wait.UntilWithContext(...)
		/root/go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/backoff.go:170
	created by github.com/openshift-online/maestro/cmd/maestro/server.(*HealthCheckServer).Start in goroutine 122
		/maestro/cmd/maestro/server/healthcheck_server.go:60 +0x13e
 >
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x60 pc=0x22aad40]

)

func addLastHeartBeatAndReadyColumnInServerInstancesTable() *gormigrate.Migration {
type ServerInstance struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need Model here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No we don't need the Model.

@clyang82
Copy link
Contributor

/retest

1 similar comment
@clyang82
Copy link
Contributor

/retest

Copy link
Contributor

@clyang82 clyang82 left a comment

Choose a reason for hiding this comment

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

LGTM

@clyang82 clyang82 merged commit eddc46d into openshift-online:main Dec 17, 2024
6 of 7 checks passed
@machi1990 machi1990 deleted the fix/migration branch December 17, 2024 15:11
machi1990 added a commit to Azure/ARO-HCP that referenced this pull request Dec 17, 2024
This includes a change that fix a migration issue (openshift-online/maestro#233) observed from the previous bump #998
machi1990 added a commit to Azure/ARO-HCP that referenced this pull request Dec 17, 2024
This includes a change that fix a migration issue (openshift-online/maestro#233) observed from the previous bump #998
geoberle pushed a commit to Azure/ARO-HCP that referenced this pull request Dec 17, 2024
…57 (#1003)

This includes a change that fix a migration issue (openshift-online/maestro#233) observed from the previous bump #998
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.

3 participants