Add UT for route agent health checker

[aswinsuryan]

Add UT for route agent health checker

[submariner-bot]

🤖 Created branch: z_pr3152/aswinsuryan/route-agent-health-checker-ut
🚀 Full E2E won't run until the "ready-to-test" label is applied. I will add it automatically once the PR has 2 approvals, or you can add it manually.

[tpantelis]

I think we should structure the test specs as follows:

var _ = Describe("RouteAgent syncing", func() {
    It("should create a RouteAgent resource", func() {
         // await the RouteAgent - no RemoteEndpoint info expected
    })

    When("a remote Endpoint is created", func() {
        It("should add its RemoteEndpoint information to the RouteAgent resource", func() {
            // await the RemoteEndpoint and verify the Spec
        })
    })

    When("a remote Endpoint is updated", func() {
        It("should update its RemoteEndpoint information in the RouteAgent resource", ...)

    When("a remote Endpoint is deleted", func() {
        It("should remove its RemoteEndpoint information from the RouteAgent resource", ...)
})

var _ = Describe("RemoteEndpoint latency info", func() {
    When("a remote Endpoint is created", func() {
        It("should start a pinger and correctly update the RemoteEndpoint Status and LatencyInfo", func() {
            // await the RemoteEndpoint and verify the LatencyInfo and Status
        })

        Context("with no HealthCheckIP", func() {
		It("should not start a pinger and should set the RemoteEndpoint Status to None", ...)
	})

        Context("with health check not enabled", func() {
		It("should not start a pinger and should set the RemoteEndpoint Status to None", ...)
	})

        When("on the gateway", func() {
		It("should not start a pinger and should set the RemoteEndpoint Status to None", ...)
                 // Call ControllerSupport.CreateLocalHostEndpoint() before creating the remote EP
	})
    })

    When("a remote Endpoint is updated", func() {
        Context("and the HealthCheckIP was changed", func() {
            It("should stop the pinger and start a new one", ...)
        })

        Context("and the HealthCheckIP did not change", func() {
            It("should not start a new pinger", ...)
        })
    })

    When("a remote Endpoint is deleted", func() {
        It("should stop the pinger", ...)
})

[aswinsuryan]

I think we should structure the test specs as follows:

var _ = Describe("RouteAgent syncing", func() {
    It("should create a RouteAgent resource", func() {
         // await the RouteAgent - no RemoteEndpoint info expected
    })

    When("a remote Endpoint is created", func() {
        It("should add its RemoteEndpoint information to the RouteAgent resource", func() {
            // await the RemoteEndpoint and verify the Spec
        })
    })

    When("a remote Endpoint is updated", func() {
        It("should update its RemoteEndpoint information in the RouteAgent resource", ...)

    When("a remote Endpoint is deleted", func() {
        It("should remove its RemoteEndpoint information from the RouteAgent resource", ...)
})

var _ = Describe("RemoteEndpoint latency info", func() {
    When("a remote Endpoint is created", func() {
        It("should start a pinger and correctly update the RemoteEndpoint Status and LatencyInfo", func() {
            // await the RemoteEndpoint and verify the LatencyInfo and Status
        })

        Context("with no HealthCheckIP", func() {
		It("should not start a pinger and should set the RemoteEndpoint Status to None", ...)
	})

        Context("with health check not enabled", func() {
		It("should not start a pinger and should set the RemoteEndpoint Status to None", ...)
	})

        When("on the gateway", func() {
		It("should not start a pinger and should set the RemoteEndpoint Status to None", ...)
                 // Call ControllerSupport.CreateLocalHostEndpoint() before creating the remote EP
	})
    })

    When("a remote Endpoint is updated", func() {
        Context("and the HealthCheckIP was changed", func() {
            It("should stop the pinger and start a new one", ...)
        })

        Context("and the HealthCheckIP did not change", func() {
            It("should not start a new pinger", ...)
        })
    })

    When("a remote Endpoint is deleted", func() {
        It("should stop the pinger", ...)
})

Thanks for the suggestion . I updated the structure .

[aswinsuryan]

@tpantelis there is a race condition being reported in the set state method, where do you think we need to fix it ? some of the test infra ? Or do you think it is the code itself?

WARNING: DATA RACE
Read at 0x00c0000e3860 by goroutine 21:
github.com/submariner-io/submariner/pkg/event.(*HandlerBase).State()
/go/src/github.com/submariner-io/submariner/pkg/event/handler.go:116 +0x30c
github.com/submariner-io/submariner/pkg/routeagent_driver/handlers/healthchecker.(*controller).syncRouteAgentStatus()

[tpantelis]

@tpantelis there is a race condition being reported in the set state method, where do you think we need to fix it ? some of the test infra ? Or do you think it is the code itself?

I pushed 8a30449 to address it.

[tpantelis]

I had additional minor adjustments that I just pushed in another commit rather then adding comments (thought it would be easier).

[submariner-bot]

🤖 Closed branches: [z_pr3152/aswinsuryan/route-agent-health-checker-ut]