Use VClusterOps API to get node details in podfacts #769
Conversation
pkg/controllers/vdb/podfacts.go
Outdated
| func (p *PodFacts) makeNodeInfoFetcher(vdb *vapi.VerticaDB, pf *PodFact) catalog.Fetcher { | ||
| return catalog.MakeVSQL(vdb, p.PRunner, pf.name, pf.execContainerName) | ||
| const vclusterAPISupportedMinVersion = "v24.3.0" | ||
| vdbVer, ok := vdb.GetVerticaVersionStr() |
There was a problem hiding this comment.
There is already a function for getting the version from a vdb: MakeVersionInfo.
pkg/controllers/vdb/podfacts.go
Outdated
| func (p *PodFacts) makeNodeInfoFetcher(vdb *vapi.VerticaDB, pf *PodFact) catalog.Fetcher { | ||
| return catalog.MakeVSQL(vdb, p.PRunner, pf.name, pf.execContainerName) | ||
| const vclusterAPISupportedMinVersion = "v24.3.0" |
There was a problem hiding this comment.
Can you call this const FetchNodeDetailsWithVclusterOpsMinVersion and add it to v1/version.go?
| @@ -46,7 +46,7 @@ var _ = Describe("obj_reconcile", func() { | |||
|
|
|||
| runReconciler := func(vdb *vapi.VerticaDB, expResult ctrl.Result, mode ObjReconcileModeType) { | |||
| // Create any dependent objects for the CRD. | |||
| pfacts := MakePodFacts(vdbRec, &cmds.FakePodRunner{}, logger) | |||
| pfacts := MakePodFacts(vdbRec, &cmds.FakePodRunner{}, logger, "") | |||
There was a problem hiding this comment.
| pfacts := MakePodFacts(vdbRec, &cmds.FakePodRunner{}, logger, "") | |
| pfacts := MakePodFacts(vdbRec, &cmds.FakePodRunner{}, logger, vapi.MainCluster) |
minor: use v1.MainCluster const in place of empty sandbox name
There was a problem hiding this comment.
I think this new empty string is for the dbadmin password. It might be clear if we still used a const for it. i.e. TestEmptyPassword
There was a problem hiding this comment.
I found we had a TestPassword exists in those tests. I will reuse that one.
pkg/controllers/vdb/podfacts.go
Outdated
| @@ -852,23 +807,6 @@ func parseVerticaNodeName(stdout string) string { | |||
| return "" | |||
There was a problem hiding this comment.
It's not directly related to your PR, but I saw it when reviewing your change, can you remove parseVerticaNodeName? I think we only use it for test. We don't have any non-test operator code that relies on it.
pkg/controllers/vdb/podfacts.go
Outdated
| return catalog.MakeVSQL(vdb, p.PRunner, pf.name, pf.execContainerName) | ||
| const vclusterAPISupportedMinVersion = "v24.3.0" | ||
| vdbVer, ok := vdb.GetVerticaVersionStr() | ||
| if ok { |
There was a problem hiding this comment.
It might be good to log an entry if we cannot get the version. Just so we know why we reverted back to the old vsql behaviour.
| nodeDetails.SubclusterOid = strconv.FormatUint(vnodeDetails.SubclusterID, 10) | ||
| nodeDetails.ReadOnly = vnodeDetails.IsReadOnly | ||
| nodeDetails.SandboxName = vnodeDetails.SandboxName | ||
| nodeDetails.ShardSubscriptions = int(vnodeDetails.NumberShardSubscriptions) |
There was a problem hiding this comment.
I believe there a bit of inconsistency with this one. The HTTPS endpoint will return the shard subscriptions, including the replica shard. Where as the old method that did vsql would could the number of subscriptions and explicitly avoided the replica shard in the WHERE clause. I wonder if we can just subtract the number by 1 if it's positive?
| @@ -47,6 +47,9 @@ func (nodeDetails *NodeDetails) parseVNodeDetails(vnodeDetails *vclusterops.Node | |||
| nodeDetails.ReadOnly = vnodeDetails.IsReadOnly | |||
| nodeDetails.SandboxName = vnodeDetails.SandboxName | |||
| nodeDetails.ShardSubscriptions = int(vnodeDetails.NumberShardSubscriptions) | |||
| if nodeDetails.ShardSubscriptions > 0 { | |||
There was a problem hiding this comment.
I suggest we leave a comment so that we know why we are decrementing the subscription count.
| if nodeDetails.ShardSubscriptions > 0 { | |
| // The shard subscriptions we get from vcluster includes the replica shard. | |
| // We decrement that by one to account for that. We want to know when there | |
| // are 0 shard subscriptions in order to drive a shard rebalance. | |
| if nodeDetails.ShardSubscriptions > 0 { |
This adds a new fetcher in podfacts to fetch node details from the running database inside the pod. This new fetcher will call VClusterOps API which will send an HTTP request to the database. The new fetcher will be enabled when VclusterOps annotation is set, and Vertica server version is not older than v24.3.0. The new fetcher should be faster and more reliable than the old fetcher which will execute vsql within the pod.
This adds a new fetcher in podfacts to fetch node details from the running database inside the pod. This new fetcher will call VClusterOps API which will send an HTTP request to the database. The new fetcher will be enabled when VclusterOps annotation is set, and Vertica server version is not older than v24.3.0. The new fetcher should be faster and more reliable than the old fetcher which will execute vsql within the pod.
This adds a new fetcher in podfacts to fetch node details from the running database inside the pod. This new fetcher will call VClusterOps API which will send an HTTP request to the database. The new fetcher will be enabled when VclusterOps annotation is set, and Vertica server version is not older than v24.3.0. The new fetcher should be faster and more reliable than the old fetcher which will execute vsql within the pod.
This adds a new fetcher in podfacts to fetch node details from the running database inside the pod. This new fetcher will call VClusterOps API which will send an HTTP request to the database. The new fetcher will be enabled when VclusterOps annotation is set, and Vertica server version is not older than v24.3.0. The new fetcher should be faster and more reliable than the old fetcher which will execute vsql within the pod.
This adds a new fetcher in podfacts to fetch node details from the running database inside the pod. This new fetcher will call VClusterOps API which will send an HTTP request to the database. The new fetcher will be enabled when VclusterOps annotation is set, and Vertica server version is not older than v24.3.0. The new fetcher should be faster and more reliable than the old fetcher which will execute vsql within the pod.
This adds a new fetcher in podfacts to fetch node details from the running database inside the pod. This new fetcher will call VClusterOps API which will send an HTTP request to the database. The new fetcher will be enabled when VclusterOps annotation is set, and Vertica server version is not older than v24.3.0. The new fetcher should be faster and more reliable than the old fetcher which will execute vsql within the pod.