Dashboard: ReplicationController Detail page: Service session missing after rolling update

[keimoon]

Issue details

After rolling update, service session in ReplicationController detail page is missing.

Environment

Dashboard version: 1.1.1
Kubernetes version: 1.3.2
Operating system: CoreOS
Node.js version: v6.2.2
Go version: 1.6.2

Steps to reproduce

1. Create a ReplicationController and an associated service, for example redis:
   
2. Rolling update the ReplicationController, now the service session is missing.
   

Observed result

Service session is missing, and new label was added to the RC

Expected result

Service session must be displayed

Comments

A new label "deployment: xxxxxxxxxxxxxxxx" was added to the RC, which causes function IsSelectorMatching in types.go return false.

[bryk]

What is label selector of your service?

[keimoon]

Label selector of my service is just name: redis

[bryk]

All right, I see.

So currently services with exactly the same selector appear there. Do you propose to change this? To what? Services with a selector that is a subset? Or services with selector crossing RC selector?

[keimoon]

Because a new deployment label is added after the RC is rolling updated, my proposal is to ignore the deployment label of RC when matching service's label selector and RC's label

[bryk]

@maciaszczykm @digitalfishpond Do you have an opinion on this?

[digitalfishpond]

This seems to be quite an un-scalable condition - "ignore deployment labels when they weren't present before a rolling update". I think either of @bryk 's solutions would be workable, and would still have the desired result in the current situation.

[bryk]

Yeah, I think the solution I proposed is where we should go. To relax the matching rule to include services that are have selector that is a subset of RC's one.

[bryk]

What's the status of this?

[digitalfishpond]

my-nginx before and after rolling update are considered 2 separate RCs

In backend/common/types.go (line 178) I could reverse the logic:

instead of the current:

what do you think?

[digitalfishpond]

About to make a PR (|| changed to &&)

[bryk]

The more I look at this the more I think that current behavior is correct and has been fixed by me in https://github.com/kubernetes/dashboard/commit/bd46a59cc5dc34f2bcdf26ace36f106b0ae8de43.

Basically, we currently do subset-checking, which is correct and as expected. The bug was that we were not checking the label selector, but labels of a service. And I fixed this in aforementioned commit.

@floreks @maciaszczykm Can you validate?

[digitalfishpond]

Just checking (because my message really wasn't clear) - the top code is mine, the bottom code is how it currently stands. Currently it fails at the first difference. I make it succeed at the first similarity.

[floreks]

Let's look at this example:

selector: label-1: a, label-2: b
labelSelector: label-1: a

Result will be false because matching labelSelector against label-2: b will be invalid.

On the other hand if we have.

selector: label-1: a
labelSelector: label-1: a, label-2: b

It will return true because selector matches subset of expected labelSelector.

The question is if we should allow first situation to also pass. Expected selector is a subset of given selector and matches only part of it, but still we'll get false.

[bryk]

Currently it fails at the first difference.

And it should be like this. For a replication controller, we should show services that have exactly the same label selector, or a service has subset label selector of the RC. I think that current code does this. Do you agree? @floreks @digitalfishpond

[floreks]

I'd say that it depends on the service logic. Can existing service point to newly created Pods targeted by this new RC after rolling update? In other words, can we access new RC Pods through this existing service? If this is inherited after rolling update and we don't have to create new service then IMO we should show this service and relax those rules.

[bryk]

Situation: I have a deployment that targets labels app: my-app, env:prod. I also have a service with the same target. Then I do a rolling update, which results in a new RC that has app: my-app, env:prod, pod-template-hash: 12345. Current code will show the service, because because it has subset label selector.

If we went with "succeed at the first similarity", we'll also show services that target completely different apps, but in env:prod. This must be a subset matching, otherwise it does not make sense.

[floreks]

Of course, this is true. We may need to introduce some more complex logic than "succeed at first match". I'm not sure if existing service will allow access to new "app" after rolling update, but if this is true then we should consider how to handle that. If it's not then we can stick to current logic as it is perfectly fine.

[floreks]

I'll do quick test and get back to you. :)

[floreks]

For me original issue does not occur anymore. According to this:

The way this is intended to work is that services should select the common subset of labels across all pods they target.

This is what currently happens, so we should not change logic. We can close :)

[bryk]

Ok, cool :) Closing. Thanks for resolving this.