Skip to content
This repository has been archived by the owner on Dec 16, 2024. It is now read-only.

GH-311 gateway status instead of ocm api calls #465

Merged
merged 8 commits into from
Oct 11, 2023

Conversation

maksymvavilov
Copy link
Contributor

@maksymvavilov maksymvavilov commented Aug 31, 2023

What

Change in how we get information about clusters onto which we placed gateway. Instead of poking the OCM we now extract those from the GW CR itself.

Verification

  1. Follow one of the submariner PoCs up to the point where you have gateway placed on the multiple clusters
  2. Ensure that DNSRecords have endpoints set

closes #311

@maksymvavilov maksymvavilov temporarily deployed to e2e-internal August 31, 2023 11:13 — with GitHub Actions Inactive
@maksymvavilov maksymvavilov temporarily deployed to e2e-internal September 1, 2023 14:47 — with GitHub Actions Inactive
@maksymvavilov maksymvavilov changed the title [WIP] GH-311 gateway status instead of ocm api calls GH-311 gateway status instead of ocm api calls Sep 1, 2023
@maksymvavilov maksymvavilov marked this pull request as ready for review September 1, 2023 15:31
@maksymvavilov maksymvavilov temporarily deployed to e2e-internal September 1, 2023 15:35 — with GitHub Actions Inactive
@maksymvavilov maksymvavilov temporarily deployed to e2e-internal September 1, 2023 15:42 — with GitHub Actions Inactive
Copy link
Member

@mikenairn mikenairn left a comment

Choose a reason for hiding this comment

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

The main goal with this task was to remove the dependency on the OCM placement API in the dnspolicy controller and reconcilers. As this is, it's removed a small bit of it but not all. I'd start by removing this https://github.com/Kuadrant/multicluster-gateway-controller/blob/main/pkg/controllers/dnspolicy/dnspolicy_controller.go#L66 and see what breaks and make it work without it.

You probably need to process the status of a gateway and build up a map of clusters and it's addresses and iterate over that rather than calling the placer functions, also the attached routes info should be available in both cases directly from the gateway.

Listeners also need to be processed differently to take into account the two types of Gateway (Multi and single):

kubectl get gateway prod-web -n multi-cluster-gateways -o json | jq .status.listeners
[
  {
    "attachedRoutes": 1,
    "conditions": [],
    "name": "kind-mgc-control-plane.api",
    "supportedKinds": []
  },
  {
    "attachedRoutes": 1,
    "conditions": [],
    "name": "kind-mgc-workload-1.api",
    "supportedKinds": []
  },
  {
    "attachedRoutes": 1,
    "conditions": [],
    "name": "kind-mgc-workload-2.api",
    "supportedKinds": []
  }
]

And the object that represents a "Cluster" should be any GVK, rather than hard coded to an OCM "ManagedCluster". This is just a resource that has labels so can be anything, but will always be a "ManagedCluster" in MGC.

pkg/controllers/dnspolicy/dnspolicy_dnsrecords.go Outdated Show resolved Hide resolved
pkg/controllers/dnspolicy/dnspolicy_dnsrecords.go Outdated Show resolved Hide resolved
test/integration/dnspolicy_controller_test.go Show resolved Hide resolved
test/integration/dnspolicy_controller_test.go Outdated Show resolved Hide resolved
@maksymvavilov
Copy link
Contributor Author

/hold

@maksymvavilov maksymvavilov temporarily deployed to e2e-internal September 6, 2023 14:27 — with GitHub Actions Inactive
@maksymvavilov maksymvavilov temporarily deployed to e2e-internal September 7, 2023 14:27 — with GitHub Actions Inactive
@maksymvavilov maksymvavilov temporarily deployed to e2e-internal September 7, 2023 14:33 — with GitHub Actions Inactive
@maksymvavilov maksymvavilov temporarily deployed to e2e-internal September 7, 2023 16:12 — with GitHub Actions Inactive
@maksymvavilov
Copy link
Contributor Author

/label tide/merge-method-squash

@openshift-ci openshift-ci bot added the tide/merge-method-squash Instructing Openshift CI bot to squash commits on merge label Sep 7, 2023
@maksymvavilov maksymvavilov temporarily deployed to e2e-internal September 15, 2023 12:05 — with GitHub Actions Inactive
@maksymvavilov maksymvavilov temporarily deployed to e2e-internal September 18, 2023 12:10 — with GitHub Actions Inactive
@mikenairn mikenairn self-requested a review September 22, 2023 08:15
for _, statusListener := range upstreamGateway.Status.Listeners {
// assuming all adresses of the same type on the gateway
// for Multi Cluster (MGC Gateway Status)
if *addresses[0].Type == gateway.MultiClusterIPAddressType || *addresses[0].Type == gateway.MultiClusterHostnameAddressType {
Copy link
Member

Choose a reason for hiding this comment

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

Not ideal having to pass in all the addresses here just to work out if it's an MGC updated status we are dealing with or not. I can see why you have done it this way though, so maybe a better way of determining the type of Gateway we are dealing with, and the expected status, can be looked into later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

adding this to the stockpile of "fix later"

@maksymvavilov maksymvavilov temporarily deployed to e2e-internal October 5, 2023 11:59 — with GitHub Actions Inactive
@mikenairn mikenairn dismissed their stale review October 10, 2023 08:38

OUt of date

@mikenairn
Copy link
Member

/lgtm
/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 11, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: makslion, mikenairn

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot merged commit 8946fef into Kuadrant:main Oct 11, 2023
9 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved lgtm tide/merge-method-squash Instructing Openshift CI bot to squash commits on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update dnspolicy reconciler to use gateway status for placed cluster details
4 participants