include sessionCookieEnabled when syncing routes #121

rustyjux · 2024-10-11T23:46:57Z

Description

Fixes APS-2743 - gwa scheduler is not using the route tag aps.route.session.cookie.enabled
Includes sessionCookieEnabled when examining the existing routes in OCP on the cluster being synced

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Documentation (non-breaking change with enhancements to documentation)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist

I have checked that unit tests pass locally with my changes
I have added tests that prove my fix is effective or that my feature works
I have added necessary documentation (if appropriate)

ikethecoder · 2024-10-15T19:14:31Z

microservices/kubeApi/routers/routes.py

            }
        )

    insert_batch = [x for x in source_routes if not in_list(x, existing_routes)]
    delete_batch = [y for y in existing_routes if not in_list(y, source_routes)]

+    if namespace == "gw-41c0c":


remove code block

microservices/kubeApi/tests/routers/test_bulk_sync.py

rustyjux · 2024-10-16T18:57:16Z

Ready for review. Changes worked as expected in dev and are currently deployed there.

microservices/kubeApi/tests/routers/test_bulk_sync_session_cookie.py

ikethecoder · 2024-10-17T23:10:09Z

microservices/kubeApi/routers/routes.py

    except Exception as ex:
        traceback.print_exc()
        logger.error("Error creating routes. %s" % (ex))
        raise HTTPException(status_code=400, detail="Error creating routes. %s" % (ex))
-    except SystemExit as ex:


Are you sure it is OK to remove the raising of the SystemExit exception? I think I had to include this otherwise the SIGTERM signal is ignored if the process is in the middle of that code block. So could take longer to terminate the Pod.

My bad - I had removed earlier when trying to resolve errors that came up when deleting a route that doesn't exist (until I saw and modeled off microservices/kubeApi/tests/routers/test_del_route.py) and should have restored them.

More generally, I should have reviewed all my changes in the commit. Thanks for the catch

rustyjux · 2024-10-18T19:10:16Z

With the most recent changes in kube_api in dev (running in Gold which is in standby mode), I observed a route needing syncing being deleted and then re-inserted on the next scheduler run.

Detailed observations:

there was an existing route not-sticky-ias9df9-dev which had the aps.route.session.cookie.enabled tag (so actually was sticky). was previously synced to both clsuters
I removed the aps.route.session.cookie.enabled tag on the route in the gw config
the update of the route in GoldDR is immediate and fine
the first time sync occurs, the route is deleted in Gold and is missing - I actually observed in Route in OCP the route being absent

[2024-10-18 18:24:32,249] DEBUG routes.verify_and_create_routes: insert batch: [{'name': 'wild-ns-gw-41c0c-not-sticky-ias9df9-dev-api-gov-bc-ca.dev.api.gov.bc.ca', 'selectTag': 'ns.gw-41c0c', 'host': 'not-sticky-ias9df9-dev-api-gov-bc-ca.dev.api.gov.bc.ca', 'sessionCookieEnabled': False, 'dataPlane': 'konghd-kong-proxy'}]
[2024-10-18 18:24:32,249] DEBUG routes.verify_and_create_routes: delete batch: [{'name': 'wild-ns-gw-41c0c-not-sticky-ias9df9-dev-api-gov-bc-ca.dev.api.gov.bc.ca', 'selectTag': 'ns.gw-41c0c', 'host': 'not-sticky-ias9df9-dev-api-gov-bc-ca.dev.api.gov.bc.ca', 'dataPlane': 'konghd-kong-proxy', 'sessionCookieEnabled': True}]
[2024-10-18 18:24:32,250] DEBUG routes.verify_and_create_routes: Creating 1 routes - tmp /tmp/sync/20241018182432-4d18599f5c
[2024-10-18 18:24:33,235] DEBUG ocp_routes.prepare_apply_routes: [ns.gw-41c0c] ['not-sticky-ias9df9-dev-api-gov-bc-ca.dev.api.gov.bc.ca'] No override applied {}
[2024-10-18 18:24:33,235] DEBUG ocp_routes.prepare_apply_routes: [ns.gw-41c0c] Route A 001 matched ssl cert data-api.tls
[2024-10-18 18:24:33,235] DEBUG ocp_routes.prepare_apply_routes: [ns.gw-41c0c] Route A 001 wild-ns-gw-41c0c-not-sticky-ias9df9-dev-api-gov-bc-ca.dev.api.gov.bc.ca (ver.3542253514)
[2024-10-18 18:24:33,236] DEBUG routes.verify_and_create_routes: [gw-41c0c] - Prepared 1 routes
[2024-10-18 18:24:33,946] DEBUG routes.verify_and_create_routes: [gw-41c0c] - Applied 1 routes
[2024-10-18 18:24:33,946] DEBUG routes.verify_and_create_routes: Deleting 1 routes
[2024-10-18 18:24:34,848] DEBUG routes.verify_and_create_routes: [gw-41c0c] - Deleted route wild-ns-gw-41c0c-not-sticky-ias9df9-dev-api-gov-bc-ca.dev.api.gov.bc.ca
[2024-10-18 18:24:34,848] INFO h11_impl. send: 10.97.8.1:43580 - "POST /namespaces/gw-41c0c/routes/sync HTTP/1.1" 200

on the next sync round, the route is recreated

[2024-10-18 18:40:47,239] DEBUG routes.verify_and_create_routes: insert batch: [{'name': 'wild-ns-gw-41c0c-not-sticky-ias9df9-dev-api-gov-bc-ca.dev.api.gov.bc.ca', 'selectTag': 'ns.gw-41c0c', 'host': 'not-sticky-ias9df9-dev-api-gov-bc-ca.dev.api.gov.bc.ca', 'sessionCookieEnabled': False, 'dataPlane': 'konghd-kong-proxy'}]
[2024-10-18 18:40:47,239] DEBUG routes.verify_and_create_routes: delete batch: []
[2024-10-18 18:40:47,240] DEBUG routes.verify_and_create_routes: Creating 1 routes - tmp /tmp/sync/20241018184047-2e8e88f2aa
[2024-10-18 18:40:48,236] DEBUG ocp_routes.prepare_apply_routes: [ns.gw-41c0c] ['not-sticky-ias9df9-dev-api-gov-bc-ca.dev.api.gov.bc.ca'] No override applied {}
[2024-10-18 18:40:48,236] DEBUG ocp_routes.prepare_apply_routes: [ns.gw-41c0c] Route A 001 matched ssl cert data-api.tls
[2024-10-18 18:40:48,236] DEBUG ocp_routes.prepare_apply_routes: [ns.gw-41c0c] Route A 001 wild-ns-gw-41c0c-not-sticky-ias9df9-dev-api-gov-bc-ca.dev.api.gov.bc.ca (ver.)
[2024-10-18 18:40:48,237] DEBUG routes.verify_and_create_routes: [gw-41c0c] - Prepared 1 routes
[2024-10-18 18:40:48,841] DEBUG routes.verify_and_create_routes: [gw-41c0c] - Applied 1 routes
[2024-10-18 18:40:48,841] INFO h11_impl. send: 10.97.8.1:19364 - "POST /namespaces/gw-41c0c/routes/sync HTTP/1.1" 200

update gw config to restore the aps.route.session.cookie.enabled tag
next sync cycle the route is deleted again

[2024-10-18 18:57:02,236] DEBUG routes.verify_and_create_routes: insert batch: [{'name': 'wild-ns-gw-41c0c-not-sticky-ias9df9-dev-api-gov-bc-ca.dev.api.gov.bc.ca', 'selectTag': 'ns.gw-41c0c', 'host': 'not-sticky-ias9df9-dev-api-gov-bc-ca.dev.api.gov.bc.ca', 'sessionCookieEnabled': True, 'dataPlane': 'konghd-kong-proxy'}]
[2024-10-18 18:57:02,236] DEBUG routes.verify_and_create_routes: delete batch: [{'name': 'wild-ns-gw-41c0c-not-sticky-ias9df9-dev-api-gov-bc-ca.dev.api.gov.bc.ca', 'selectTag': 'ns.gw-41c0c', 'host': 'not-sticky-ias9df9-dev-api-gov-bc-ca.dev.api.gov.bc.ca', 'dataPlane': 'konghd-kong-proxy', 'sessionCookieEnabled': False}]
[2024-10-18 18:57:02,236] DEBUG routes.verify_and_create_routes: Creating 1 routes - tmp /tmp/sync/20241018185702-23d00a2cbe
[2024-10-18 18:57:03,136] DEBUG ocp_routes.prepare_apply_routes: [ns.gw-41c0c] Route A 001 matched ssl cert data-api.tls
[2024-10-18 18:57:03,136] DEBUG ocp_routes.prepare_apply_routes: [ns.gw-41c0c] Route A 001 wild-ns-gw-41c0c-not-sticky-ias9df9-dev-api-gov-bc-ca.dev.api.gov.bc.ca (ver.3544619441)
[2024-10-18 18:57:03,136] DEBUG routes.verify_and_create_routes: [gw-41c0c] - Prepared 1 routes
[2024-10-18 18:57:03,935] DEBUG routes.verify_and_create_routes: [gw-41c0c] - Applied 1 routes
[2024-10-18 18:57:03,935] DEBUG routes.verify_and_create_routes: Deleting 1 routes
[2024-10-18 18:57:04,833] DEBUG routes.verify_and_create_routes: [gw-41c0c] - Deleted route wild-ns-gw-41c0c-not-sticky-ias9df9-dev-api-gov-bc-ca.dev.api.gov.bc.ca
[2024-10-18 18:57:04,833] INFO h11_impl. send: 10.97.8.1:62568 - "POST /namespaces/gw-41c0c/routes/sync HTTP/1.1" 200

So why is delete now happening after insert? Presumably this was not always the behaviour, though it does come second in routes and was always based on route name

gwa-api/microservices/kubeApi/routers/routes.py

Line 217 in eaa8ea6

kubectl_delete('route', route["name"])

I have set kube api back to dev branch in versions to avoid creating more issues

ikethecoder · 2024-10-18T19:46:28Z

Hmm... I think the reason is that for deletion, it really just needs to know whether the name is no longer in the source_routes. The name is derived from the selectTag and host so this insert+delete issue would not have happened (dataPlane doesn't change). When you added sessionCookieEnabled then differences get added into the delete list incorrectly. To fix, I would just compare the name for building the delete_batch.

rustyjux · 2024-10-21T23:55:00Z

Aha thanks for digging in there. That's a tricky side effect from changing in_list() which is used in the comparison. I updated tests to reflect where deletes should and should not be happening.

Newest code works as expected when a sync update is needed in DR:

[2024-10-21 23:50:50,966] DEBUG routes.verify_and_create_routes: insert batch: [{'name': 'wild-ns-gw-41c0c-sticky-icky-ias9df9-dev-api-gov-bc-ca.dev.api.gov.bc.ca', 'selectTag': 'ns.gw-41c0c', 'host': 'sticky-icky-ias9df9-dev-api-gov-bc-ca.dev.api.gov.bc.ca', 'sessionCookieEnabled': False, 'dataPlane': 'konghd-kong-proxy'}]
[2024-10-21 23:50:50,966] DEBUG routes.verify_and_create_routes: delete batch: []
[2024-10-21 23:50:50,968] DEBUG routes.verify_and_create_routes: Creating 1 routes - tmp /tmp/sync/20241021235050-dfbc437d7c
[2024-10-21 23:50:51,956] DEBUG ocp_routes.prepare_apply_routes: [ns.gw-41c0c] ['sticky-icky-ias9df9-dev-api-gov-bc-ca.dev.api.gov.bc.ca'] No override applied {}
[2024-10-21 23:50:51,956] DEBUG ocp_routes.prepare_apply_routes: [ns.gw-41c0c] Route A 001 matched ssl cert data-api.tls
[2024-10-21 23:50:51,956] DEBUG ocp_routes.prepare_apply_routes: [ns.gw-41c0c] Route A 001 wild-ns-gw-41c0c-sticky-icky-ias9df9-dev-api-gov-bc-ca.dev.api.gov.bc.ca (ver.2834629484)
[2024-10-21 23:50:51,956] DEBUG routes.verify_and_create_routes: [gw-41c0c] - Prepared 1 routes
[2024-10-21 23:50:52,585] DEBUG routes.verify_and_create_routes: [gw-41c0c] - Applied 1 routes
[2024-10-21 23:50:52,585] INFO h11_impl. send: 10.95.10.1:14778 - "POST /namespaces/gw-41c0c/routes/sync HTTP/1.1" 200

rustyjux · 2024-10-22T14:44:57Z

Ugh probably should have squash merged. Could do a hard reset or we live with it

rustyjux added 4 commits October 11, 2024 16:06

pull sessionCookieEnabled from kube admin api data when syncing routes

ee3e55f

update test

c9752a1

fix test data

f4ac303

handle if aps-template-version is not provided

5071faa

rustyjux requested review from ikethecoder and Elson9 October 11, 2024 23:46

rustyjux added 3 commits October 15, 2024 08:57

debug logging

8b8bfa5

update python v in gatewayApi

c27782b

commit updated lock file

8eeee61

ikethecoder requested changes Oct 15, 2024

View reviewed changes

rustyjux and others added 7 commits October 15, 2024 12:24

clean poetry env before tests

78fcdc3

revert to 3.11 python for gwa api

a0fa63c

revert to 3.11 python for gwa api

d8d9189

review python env

50d68d5

review python env

41217f6

review python env

ecf71d3

upd dockerfile to 3.11

8656e18

rustyjux force-pushed the feature/sync-route-template-v branch from b65f165 to 8656e18 Compare October 16, 2024 15:31

rustyjux added the wip label Oct 16, 2024

rustyjux added 3 commits October 16, 2024 11:04

include sessionCookieEnabled in comparison

7fdc717

fix build_ref, add test for sessionCookieEnabled false

dc2f68e

remove extra debug logging

3329a79

rustyjux removed the wip label Oct 16, 2024

ikethecoder requested changes Oct 17, 2024

View reviewed changes

microservices/kubeApi/tests/routers/test_bulk_sync_session_cookie.py Outdated Show resolved Hide resolved

return counts for inserted and deleted from routes/sync

9699327

ikethecoder reviewed Oct 17, 2024

View reviewed changes

restore exception handling

fabbbb9

rustyjux added the wip label Oct 18, 2024

only check route name for building delete list

e8ac803

rustyjux removed the wip label Oct 21, 2024

ikethecoder approved these changes Oct 22, 2024

View reviewed changes

rustyjux merged commit 4890602 into dev Oct 22, 2024
7 checks passed

rustyjux deleted the feature/sync-route-template-v branch October 23, 2024 00:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

include sessionCookieEnabled when syncing routes #121

include sessionCookieEnabled when syncing routes #121

rustyjux commented Oct 11, 2024

ikethecoder Oct 15, 2024 •

edited

Loading

rustyjux commented Oct 16, 2024

ikethecoder Oct 17, 2024

rustyjux Oct 18, 2024

rustyjux commented Oct 18, 2024

ikethecoder commented Oct 18, 2024

rustyjux commented Oct 21, 2024

rustyjux commented Oct 22, 2024

include sessionCookieEnabled when syncing routes #121

include sessionCookieEnabled when syncing routes #121

Conversation

rustyjux commented Oct 11, 2024

Description

Types of changes

Checklist

ikethecoder Oct 15, 2024 • edited Loading

Choose a reason for hiding this comment

rustyjux commented Oct 16, 2024

ikethecoder Oct 17, 2024

Choose a reason for hiding this comment

rustyjux Oct 18, 2024

Choose a reason for hiding this comment

rustyjux commented Oct 18, 2024

ikethecoder commented Oct 18, 2024

rustyjux commented Oct 21, 2024

rustyjux commented Oct 22, 2024

ikethecoder Oct 15, 2024 •

edited

Loading