Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Streamline Gslb Status #116

Closed
ytsarev opened this issue May 11, 2020 · 4 comments · Fixed by #121
Closed

Streamline Gslb Status #116

ytsarev opened this issue May 11, 2020 · 4 comments · Fixed by #121
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@ytsarev
Copy link
Member

ytsarev commented May 11, 2020

Currently we have Gslb Status content as shown in example below:

  status:
    healthyRecords:
      app3.cloud.example.com:
      - 172.17.0.4
      - 172.17.0.5
      - 172.17.0.6
    managedHosts:
    - app1.cloud.example.com
    - app2.cloud.example.com
    - app3.cloud.example.com
    serviceHealth:
      app1.cloud.example.com: NotFound
      app2.cloud.example.com: Unhealthy
      app3.cloud.example.com: Healthy

It is easily visible that we have duplication between the managedHosts and serviceHealth
It happened historically during development of the project. Initially serviceHealth looked like

    serviceHealth:
      unhealthyServiceName: NotFound
      backend: Unhealthy
      frontend: Healthy

so basically referenced serviceName that is referenced in associated gslb ingress.
Later on it was changed to ingressHost:Status as it became apparent that it is practical to have them in a single data structure.

So some questions to discuss:

  1. Do we need serviceName exposed somewhere in Status ?
  2. If yes - should we extend serviceHealth or place it somewhere else ?
  3. Easiest scenario - we just remove managedHosts
@ytsarev ytsarev added the enhancement New feature or request label May 11, 2020
@ytsarev ytsarev added this to the 0.7 milestone May 11, 2020
@kuritka
Copy link
Collaborator

kuritka commented May 11, 2020

Hi true is that managedHosts looks a little redundant.

Perhaps this ?

  status:
    serviceHealth:
      app1.cloud.example.com: NotFound
      app2.cloud.example.com: Unhealthy
      app3.cloud.example.com: Healthy

    healthyRecords:
      app3.cloud.example.com:
      - 172.17.0.4
      - 172.17.0.5
      - 172.17.0.6

@ytsarev
Copy link
Member Author

ytsarev commented May 11, 2020

Yeah, so it is 3)

@ytsarev
Copy link
Member Author

ytsarev commented May 11, 2020

will implement it removal. We can always amend more data whenever we need anything else to be exposed in Status

@ytsarev
Copy link
Member Author

ytsarev commented May 12, 2020

Another issues with the Status. Whenever workload on the specific cluster is down it does not populate HealthyRecords with external cluster records even if it is alive. Meanwhile in reality it populates response with external cluster entries.

@ytsarev ytsarev self-assigned this May 14, 2020
ytsarev added a commit that referenced this issue May 14, 2020
* Remove redundant `ManagedHosts`
* Show external IP addresses in `HealthyRecords` even if local service
  is `Unhealthy`
* Expose `GeoTag`
* Resolves #116
@ytsarev ytsarev modified the milestones: 0.7, 0.6 May 14, 2020
ytsarev added a commit that referenced this issue May 15, 2020
* Remove redundant `ManagedHosts`
* Show external IP addresses in `HealthyRecords` even if local service
  is `Unhealthy`
* Expose `GeoTag`
* Resolves #116
ytsarev added a commit that referenced this issue May 15, 2020
* Remove redundant `ManagedHosts`
* Show external IP addresses in `HealthyRecords` even if local service
  is `Unhealthy`
* Expose `GeoTag`
* Resolves #116
ytsarev added a commit that referenced this issue May 15, 2020
* Remove redundant `ManagedHosts`
* Show external IP addresses in `HealthyRecords` even if local service
  is `Unhealthy`
* Expose `GeoTag`
* Resolves #116
ytsarev added a commit that referenced this issue May 15, 2020
* Remove redundant `ManagedHosts`
* Show external IP addresses in `HealthyRecords` even if local service
  is `Unhealthy`
* Expose `GeoTag`
* Resolves #116
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants