Add controller-isleader xweb component to respond with 200 if the controller in question is a raft leader

[nenkoru]

closes(backref) #2582

[nenkoru]

moved from /sys/health to a /controller/health with changes made to make it more generic

[plorenz]

Hi @nenkoru , before going too deep with this PR, we require a CLA before accepting contributions. Information on that is here: https://openziti.io/policies/CONTRIBUTING.html#code-contributions
Let us know if you're ok with submitting that.

[nenkoru]

Hi @nenkoru , before going too deep with this PR, we require a CLA before accepting contributions. Information on that is here: https://openziti.io/policies/CONTRIBUTING.html#code-contributions Let us know if you're ok with submitting that.

Hey, @plorenz!
Sent a signed ICLA to cla@netfoundry.io yesterday.
Can't wait for processing to complete!

[plorenz]

Hi @nenkoru , before going too deep with this PR, we require a CLA before accepting contributions. Information on that is here: https://openziti.io/policies/CONTRIBUTING.html#code-contributions Let us know if you're ok with submitting that.

Hey, @plorenz! Sent a signed ICLA to cla@netfoundry.io yesterday. Can't wait for processing to complete!

CLA looks good 👍

Having looked at it again, I think it looks overall. The only 2 thing I'd like changed are:

1. A flag to disable the 429 return so by default the results will be backwards compabible
2. This should replace the existing health checker, rather than add to it. I'd rather have one flexible endpoint than multiple, since they are largely the same.

Let me know if those make sense.

[nenkoru]

Hi @nenkoru , before going too deep with this PR, we require a CLA before accepting contributions. Information on that is here: https://openziti.io/policies/CONTRIBUTING.html#code-contributions Let us know if you're ok with submitting that.

Hey, @plorenz! Sent a signed ICLA to cla@netfoundry.io yesterday. Can't wait for processing to complete!

CLA looks good 👍

Having looked at it again, I think it looks overall. The only 2 thing I'd like changed are:

1. A flag to disable the 429 return so by default the results will be backwards compabible
2. This should replace the existing health checker, rather than add to it. I'd rather have one flexible endpoint than multiple, since they are largely the same.

Let me know if those make sense.

I think it gets complicated with moving this logic to a health-checks handler.
We need that controller env within a healtcheck, so we have to pass it into the structure of health-check, make it depend on the controller/env.
But router.go https://github.com/openziti/ziti/blob/main/router/router.go#L274 uses the same health-check and we have to pass the env, but it's impossible due to different types (controller vs router)

We might also opt for moving this heath-check to the actual gosundheit custom check https://github.com/openziti/ziti/blob/main/controller/health.go#L15, and try to find the result of the check within that health-check loop at https://github.com/openziti/ziti/blob/main/common/health/handler.go#L99 and decide whether to return 429 status code there.

Or we could just make health-checks specific for controller.

Thoughts?

[plorenz]

Or we could just make health-checks specific for controller.

Yes, this was the direction I was thinking. Leave the router health check as it is and replace the use of the common health checker with this, controller specific one.

So the only changes required would be to

1. remove the old of the existing health checker for the controller
2. ensure that the new health check code is backwards compatible (uses the same paths, returns, etc, unless an indicator (request param or path) says that we should do the 429 return if not leader

[nenkoru]

Or we could just make health-checks specific for controller.

Yes, this was the direction I was thinking. Leave the router health check as it is and replace the use of the common health checker with this, controller specific one.

So the only changes required would be to

1. remove the old of the existing health checker for the controller
2. ensure that the new health check code is backwards compatible (uses the same paths, returns, etc, unless an indicator (request param or path) says that we should do the 429 return if not leader

Made changes accordingly. Naming was hard, but I named this as enableRaftControllerCheck. This flag makes it to do two things: Return 429 if not a raft leader and raft is enabled & Return isLeader and isRaftEnabled within raft. Making this essentially a feature flag.

Eg response:

< HTTP/1.1 429 Too Many Requests
< Content-Type: application/json
< Date: Tue, 24 Dec 2024 21:47:33 GMT
< Content-Length: 408
<
{
    "data": {
        "checks": [
            {
                "details": null,
                "healthy": true,
                "id": "bolt.read",
                "lastCheckDuration": "17.334µs",
                "lastCheckTime": "2024-12-24T21:47:24Z"
            }
        ],
        "healthy": true
    },
    "meta": {},
    "raft": {
        "isLeader": false,
        "isRaftEnabled": true
    }
}

Also I skimmed the zititest but couldn't an appropriate place to put tests for this behaviour.

I guess after tests created I could unmark this PR as being a draft and we could continue iterating on this!
Tell me what you think.