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

Listing active targets shows even inactive ones #2306

Closed
subnetmarco opened this issue Mar 30, 2017 · 6 comments
Closed

Listing active targets shows even inactive ones #2306

subnetmarco opened this issue Mar 30, 2017 · 6 comments

Comments

@subnetmarco
Copy link
Member

subnetmarco commented Mar 30, 2017

The :8001/upstreams/{name}/targets/active/ endpoint is showing both active and inactive targets when the same Target's target (is being added multiple times).

To replicate:

$ http POST :8001/upstreams/test/targets target=1.1.1.1:80
HTTP/1.1 201 Created
Access-Control-Allow-Credentials: false
Access-Control-Allow-Origin: *
Connection: keep-alive
Content-Type: application/json; charset=utf-8
Date: Thu, 30 Mar 2017 21:18:25 GMT
Server: kong/0.10.1
Transfer-Encoding: chunked

{
    "created_at": 1490908705822,
    "id": "97349d29-3595-4ff2-adae-7b8526237ce2",
    "target": "1.1.1.1:80",
    "upstream_id": "d9ec9bda-bcc6-437a-a56f-14893864eccc",
    "weight": 100
}

$ http POST :8001/upstreams/test/targets target=1.1.1.1:80
HTTP/1.1 201 Created
Access-Control-Allow-Credentials: false
Access-Control-Allow-Origin: *
Connection: keep-alive
Content-Type: application/json; charset=utf-8
Date: Thu, 30 Mar 2017 21:18:27 GMT
Server: kong/0.10.1
Transfer-Encoding: chunked

{
    "created_at": 1490908707134,
    "id": "efcad7e8-9f59-4366-8ee6-5bd406f06c75",
    "target": "1.1.1.1:80",
    "upstream_id": "d9ec9bda-bcc6-437a-a56f-14893864eccc",
    "weight": 100
}

$ http POST :8001/upstreams/test/targets target=1.1.1.1:80 weight=50
HTTP/1.1 201 Created
Access-Control-Allow-Credentials: false
Access-Control-Allow-Origin: *
Connection: keep-alive
Content-Type: application/json; charset=utf-8
Date: Thu, 30 Mar 2017 21:18:34 GMT
Server: kong/0.10.1
Transfer-Encoding: chunked

{
    "created_at": 1490908714486,
    "id": "7af6fdc1-7940-476a-82c5-28a0c691fe95",
    "target": "1.1.1.1:80",
    "upstream_id": "d9ec9bda-bcc6-437a-a56f-14893864eccc",
    "weight": 50
}

Listing active targets shows all three (instead of only one):

$ http GET :8001/upstreams/test/targets/active/
HTTP/1.1 200 OK
Access-Control-Allow-Credentials: false
Access-Control-Allow-Origin: *
Connection: keep-alive
Content-Type: application/json; charset=utf-8
Date: Thu, 30 Mar 2017 21:19:53 GMT
Server: kong/0.10.1
Transfer-Encoding: chunked

{
    "data": [
        {
            "created_at": 1490908714486,
            "id": "7af6fdc1-7940-476a-82c5-28a0c691fe95",
            "target": "1.1.1.1:80",
            "upstream_id": "d9ec9bda-bcc6-437a-a56f-14893864eccc",
            "weight": 50
        },
        {
            "created_at": 1490908707134,
            "id": "efcad7e8-9f59-4366-8ee6-5bd406f06c75",
            "target": "1.1.1.1:80",
            "upstream_id": "d9ec9bda-bcc6-437a-a56f-14893864eccc",
            "weight": 100
        },
        {
            "created_at": 1490908705822,
            "id": "97349d29-3595-4ff2-adae-7b8526237ce2",
            "target": "1.1.1.1:80",
            "upstream_id": "d9ec9bda-bcc6-437a-a56f-14893864eccc",
            "weight": 100
        }
    ],
    "total": 3
}
@p0pr0ck5
Copy link
Contributor

We defined "active" targets as those that have 0 weight. In that sense, none of these aren't "active".

Sounds like we need to change our definition of active :)

@p0pr0ck5
Copy link
Contributor

(btw this is another headache resulting from the fact that we don't abstract target history away from target objects. makes dealing with this interface a pain)

@Tieske
Copy link
Member

Tieske commented Mar 30, 2017

yes, we should abstract it away, but that will be in the new caching layer (see comments in quip docs)

active records are the last (in time) record for each target. And if that last one has weight 0, it is no longer active

@subnetmarco
Copy link
Member Author

subnetmarco commented Mar 31, 2017

active records are the last (in time) record for each target. And if that last one has weight 0, it is no longer active

Seems like we have redefined "active" with the above description. Do we all agree?

@p0pr0ck5
Copy link
Contributor

WFM, workin on this now.

@p0pr0ck5
Copy link
Contributor

Handled in #2310

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants