Add the service load balancer controller to Origin by smarterclayton · Pull Request #8633 · openshift/origin

smarterclayton · 2016-04-26T14:31:07Z

ncdc · 2016-04-26T14:38:32Z

LGTM

pweil- · 2016-04-26T15:00:10Z

pkg/cmd/server/start/start_master.go

 		kc.RunPersistentVolumeClaimRecycler(oc.ImageFor("recycler"), recyclerClient, oc.Options.PolicyConfig.OpenShiftInfrastructureNamespace)
 		kc.RunGCController(gcClient)

+		kc.RunServiceLoadBalancerController()


We've been making sure new controllers run with a service account client a la oc.GetServiceAccountClients(bootstrappolicy.InfraDaemonSetControllerServiceAccountName). Probably want it here too.

cc @deads2k

I was going to ask about that, but I looked at several other controllers in this file and saw they all used clientadapter.FromUnversionedClient(c.KubeClient). Maybe I didn't look thoroughly enough.

Yeah, there's still a mix of the old and new implementations. Been trying to catch anything new going in.

Yeah, there's still a mix of the old and new implementations. Been trying to catch anything new going in.

Agree.

As a note for whoever does the rebase, we need to sort out what we do about shared cache ACL-ing. @liggitt

Have we taken into account how we'll change this code path to use versioned
clients?

On Tue, Apr 26, 2016 at 11:04 AM, David Eads notifications@github.com
wrote:

In pkg/cmd/server/start/start_master.go
#8633 (comment):

@@ -613,6 +613,8 @@ func startControllers(oc *origin.MasterConfig, kc *kubernetes.MasterConfig) erro
kc.RunPersistentVolumeClaimRecycler(oc.ImageFor("recycler"), recyclerClient, oc.Options.PolicyConfig.OpenShiftInfrastructureNamespace)
kc.RunGCController(gcClient)

kc.RunServiceLoadBalancerController()

Yeah, there's still a mix of the old and new implementations. Been trying
to catch anything new going in.

Agree.

As a note for whoever does the rebase, we need to sort out what we do
about shared cache ACL-ing. @liggitt https://github.com/liggitt

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
https://github.com/openshift/origin/pull/8633/files/a7a264ec9a036439b5959ec6c5d2d2234057ed81#r61102578

One role per controller? Yuck

On Tue, Apr 26, 2016 at 11:31 AM, Clayton Coleman ccoleman@redhat.com
wrote:

Have we taken into account how we'll change this code path to use
versioned clients?

On Tue, Apr 26, 2016 at 11:04 AM, David Eads notifications@github.com
wrote:

In pkg/cmd/server/start/start_master.go
#8633 (comment):

@@ -613,6 +613,8 @@ func startControllers(oc *origin.MasterConfig, kc *kubernetes.MasterConfig) erro
kc.RunPersistentVolumeClaimRecycler(oc.ImageFor("recycler"), recyclerClient, oc.Options.PolicyConfig.OpenShiftInfrastructureNamespace)
kc.RunGCController(gcClient)

kc.RunServiceLoadBalancerController()

Yeah, there's still a mix of the old and new implementations. Been trying
to catch anything new going in.

Agree.

As a note for whoever does the rebase, we need to sort out what we do
about shared cache ACL-ing. @liggitt https://github.com/liggitt

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
https://github.com/openshift/origin/pull/8633/files/a7a264ec9a036439b5959ec6c5d2d2234057ed81#r61102578

One role per thing with discrete and predictable permissions to change API
objects across all namespaces? That seems pretty reasonable to me.

On Tue, Apr 26, 2016 at 11:33 AM, Clayton Coleman notifications@github.com
wrote:

In pkg/cmd/server/start/start_master.go
#8633 (comment):

@@ -613,6 +613,8 @@ func startControllers(oc *origin.MasterConfig, kc *kubernetes.MasterConfig) erro
kc.RunPersistentVolumeClaimRecycler(oc.ImageFor("recycler"), recyclerClient, oc.Options.PolicyConfig.OpenShiftInfrastructureNamespace)
kc.RunGCController(gcClient)

kc.RunServiceLoadBalancerController()

One role per controller? Yuck
… <#m_-4805712814360026323_>
On Tue, Apr 26, 2016 at 11:31 AM, Clayton Coleman _@.> wrote: >
Have we taken into account how we'll change this code path to use >
versioned clients? > > On Tue, Apr 26, 2016 at 11:04 AM, David Eads
*__@_.***> > wrote: > In pkg/cmd/server/start/start_master.go > <#8633
(comment)
https://github.com/openshift/origin/pull/8633#discussion_r61102578>: >

@@ -613,6 +613,8 @@ func startControllers(oc *origin.MasterConfig, kc
*kubernetes.MasterConfig) erro > >
kc.RunPersistentVolumeClaimRecycler(oc.ImageFor("recycler"),
recyclerClient, oc.Options.PolicyConfig.OpenShiftInfrastructureNamespace) >
kc.RunGCController(gcClient) > > > > +
kc.RunServiceLoadBalancerController() > > Yeah, there's still a mix of the
old and new implementations. Been trying > to catch anything new going in.
Agree. > > As a note for whoever does the rebase, we need to sort out
what we do > about shared cache ACL-ing. @liggitt
https://github.com/liggitt https://github.com/liggitt > > — > You are
receiving this because you authored the thread. > Reply to this email
directly or view it on GitHub > <
https://github.com/openshift/origin/pull/8633/files/a7a264ec9a036439b5959ec6c5d2d2234057ed81#r61102578>

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/openshift/origin/pull/8633/files/a7a264ec9a036439b5959ec6c5d2d2234057ed81#r61108219

I meant one service account per controller.

On Tue, Apr 26, 2016 at 12:35 PM, David Eads notifications@github.com
wrote:

In pkg/cmd/server/start/start_master.go
#8633 (comment):

@@ -613,6 +613,8 @@ func startControllers(oc *origin.MasterConfig, kc *kubernetes.MasterConfig) erro
kc.RunPersistentVolumeClaimRecycler(oc.ImageFor("recycler"), recyclerClient, oc.Options.PolicyConfig.OpenShiftInfrastructureNamespace)
kc.RunGCController(gcClient)

kc.RunServiceLoadBalancerController()

One role per thing with discrete and predictable permissions to change API
objects across all namespaces? That seems pretty reasonable to me.
… <#m_-8016314581085398451_>
On Tue, Apr 26, 2016 at 11:33 AM, Clayton Coleman _@.> wrote: In
pkg/cmd/server/start/start_master.go <#8633 (comment)
https://github.com/openshift/origin/pull/8633#discussion_r61108219>: >
@@ -613,6 +613,8 @@ func startControllers(oc *origin.MasterConfig, kc
*kubernetes.MasterConfig) erro >
kc.RunPersistentVolumeClaimRecycler(oc.ImageFor("recycler"),
recyclerClient, oc.Options.PolicyConfig.OpenShiftInfrastructureNamespace) >
kc.RunGCController(gcClient) > > + kc.RunServiceLoadBalancerController()
One role per controller? Yuck … <#m_-4805712814360026323_> On Tue, Apr 26,
2016 at 11:31 AM, Clayton Coleman *__@_.> wrote: > Have we taken into
account how we'll change this code path to use > versioned clients? > > On
Tue, Apr 26, 2016 at 11:04 AM, David Eads *__@.***> > wrote: > In
pkg/cmd/server/start/start_master.go > <#8633
#8633 (comment) <#8633 (comment)
https://github.com/openshift/origin/pull/8633#discussion_r61102578>>: >

@@ -613,6 +613,8 @@ func startControllers(oc *origin.MasterConfig, kc
*kubernetes.MasterConfig) erro > >
kc.RunPersistentVolumeClaimRecycler(oc.ImageFor("recycler"),
recyclerClient, oc.Options.PolicyConfig.OpenShiftInfrastructureNamespace) >
kc.RunGCController(gcClient) > > > > +
kc.RunServiceLoadBalancerController() > > Yeah, there's still a mix of the
old and new implementations. Been trying > to catch anything new going in.
Agree. > > As a note for whoever does the rebase, we need to sort out
what we do > about shared cache ACL-ing. @liggitt
https://github.com/liggitt https://github.com/liggitt <
https://github.com/liggitt> > > — > You are receiving this because you
authored the thread. > Reply to this email directly or view it on GitHub >
<
https://github.com/openshift/origin/pull/8633/files/a7a264ec9a036439b5959ec6c5d2d2234057ed81#r61102578>
— You are receiving this because you were mentioned. Reply to this email
directly or view it on GitHub <
https://github.com/openshift/origin/pull/8633/files/a7a264ec9a036439b5959ec6c5d2d2234057ed81#r61108219>

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
https://github.com/openshift/origin/pull/8633/files/a7a264ec9a036439b5959ec6c5d2d2234057ed81#r61119394

smarterclayton · 2016-05-01T04:49:20Z

[test]

smarterclayton · 2016-05-03T02:06:35Z

More feedback?

pweil- · 2016-05-03T11:57:51Z

none here, LGTM

openshift-bot · 2016-05-03T16:55:16Z

Evaluated for origin test up to 7609d9b

openshift-bot · 2016-05-03T17:45:20Z

continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/3555/)

smarterclayton · 2016-05-04T04:16:49Z

[merge]

openshift-bot · 2016-05-04T04:20:21Z

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/5803/) (Image: devenv-rhel7_4093)

openshift-bot · 2016-05-04T04:20:21Z

Evaluated for origin merge up to 7609d9b

pweil- reviewed Apr 26, 2016
View reviewed changes

smarterclayton force-pushed the add_service_controller branch from a7a264e to b23b7ba Compare April 26, 2016 15:56

Add the service load balancer controller to Origin

7609d9b

smarterclayton force-pushed the add_service_controller branch from b23b7ba to 7609d9b Compare May 3, 2016 16:54

openshift-bot merged commit d6879a6 into openshift:master May 4, 2016

Conversation

smarterclayton commented Apr 26, 2016

Uh oh!

ncdc commented Apr 26, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smarterclayton commented May 1, 2016

Uh oh!

smarterclayton commented May 3, 2016

Uh oh!

pweil- commented May 3, 2016

Uh oh!

openshift-bot commented May 3, 2016

Uh oh!

openshift-bot commented May 3, 2016

Uh oh!

smarterclayton commented May 4, 2016

Uh oh!

openshift-bot commented May 4, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-bot commented May 4, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

openshift-bot commented May 4, 2016 •

edited

Loading