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

ContainerServicePortConfig ems_ref not unique #35

Closed
cben opened this issue Jun 6, 2017 · 5 comments
Closed

ContainerServicePortConfig ems_ref not unique #35

cben opened this issue Jun 6, 2017 · 5 comments

Comments

@cben
Copy link
Contributor

cben commented Jun 6, 2017

Found by @Ladas.
We set :ems_ref => "#{service_id}_#{port_config.port}_#{port_config.targetPort}".
Protocol is "TCP" / "UDP", and can have both on same ports. Here is an easy repro, we probably all have these:

> oc get service kubernetes -o json
...
    "metadata": {
        "uid": "fa61ef76-e6f7-11e6-a348-001a4a162683"
...
    "spec": {
        "ports": [
            {
                "name": "https",
                "port": 443,
                "protocol": "TCP",
                "targetPort": 443
            },
            {
                "name": "dns",
                "port": 53,
                "protocol": "UDP",
                "targetPort": 8053
            },
            {
                "name": "dns-tcp",
                "port": 53,
                "protocol": "TCP",
                "targetPort": 8053
            }
        ],
...

Can disambiguate by protocol.
Perhaps name would also work Nope, name is optional (https://kubernetes.io/docs/concepts/services-networking/service/; saw heapster service with unnamed port).

@cben
Copy link
Contributor Author

cben commented Jun 6, 2017

@Ladas I do see 2 separate ContainerServicePortConfig records, with save_inventory and I think with graph refresh too.
Remind me, in which scenario do we expect this to cause a user-visible bug?

(save_container_port_configs_inventory uses only ems_ref as the find key.)

@Ladas
Copy link
Contributor

Ladas commented Jun 6, 2017

@cben unless I am mistaken, the refresh would keep creating&deleting the duplicate ports, probably every other refresh would be create and other delete. But I would have to try to know for sure. :-) And it could randomly update the data with tcp or udp, since any of those would match.

@simon3z
Copy link
Contributor

simon3z commented Jun 7, 2017

@cben @Ladas can you open a BZ? I feel we should resolve this before the graph refresh (in the current save_inventory).

@miq-bot
Copy link
Member

miq-bot commented Dec 11, 2017

This issue has been automatically marked as stale because it has not been updated for at least 6 months.

If you can still reproduce this issue on the current release or on master, please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions!

@cben
Copy link
Contributor Author

cben commented Apr 25, 2018

long fixed on gaprindashvili and master.
graph refresh fixed long ago, too lazy to find now.
classic refresh fixed by ManageIQ/manageiq#16454
tests for all modes in ManageIQ/manageiq-providers-openshift#86

@cben cben closed this as completed Apr 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants