Run the TSB in a separate pod #15672
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,12 +15,15 @@ import ( | |
| "k8s.io/apimachinery/pkg/util/wait" | ||
| genericapiserver "k8s.io/apiserver/pkg/server" | ||
| restclient "k8s.io/client-go/rest" | ||
| "k8s.io/client-go/tools/cache" | ||
| "k8s.io/kubernetes/pkg/api" | ||
| kclientsetinternal "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" | ||
| "k8s.io/kubernetes/pkg/controller" | ||
|
|
||
| "github.com/openshift/origin/pkg/cmd/server/bootstrappolicy" | ||
| templateapi "github.com/openshift/origin/pkg/template/apis/template" | ||
| templateinformer "github.com/openshift/origin/pkg/template/generated/informers/internalversion" | ||
| templateinternalclientset "github.com/openshift/origin/pkg/template/generated/internalclientset" | ||
| templateservicebroker "github.com/openshift/origin/pkg/template/servicebroker" | ||
| ) | ||
|
|
||
|
|
@@ -51,7 +54,7 @@ type TemplateServiceBrokerConfig struct { | |
|
|
||
| // PrivilegedKubeClientConfig is *not* a loopback config, since it needs to point to the kube apiserver | ||
| // TODO remove this and use the SA that start us instead of trying to cyclically find an SA token | ||
| PrivilegedKubeClientConfig *restclient.Config | ||
|
|
||
| TemplateInformers templateinformer.SharedInformerFactory | ||
| TemplateNamespaces []string | ||
|
|
@@ -87,11 +90,55 @@ func (c completedTemplateServiceBrokerConfig) New(delegationTarget genericapiser | |
| GenericAPIServer: genericServer, | ||
| } | ||
|
|
||
| inCluster := false | ||
| var broker *templateservicebroker.Broker | ||
| // TODO, this block drops out after the server is moved. | ||
| if c.PrivilegedKubeClientConfig != nil { | ||
| broker = templateservicebroker.DeprecatedNewBrokerInsideAPIServer( | ||
| *c.PrivilegedKubeClientConfig, | ||
| c.TemplateInformers.Template().InternalVersion().Templates(), | ||
| c.TemplateNamespaces, | ||
| ) | ||
| // make sure no one else uses it | ||
| c.TemplateInformers = nil | ||
|
|
||
| } else { | ||
| // we're running in cluster | ||
| // in this case we actually want to construct our own template informer | ||
| // eventually the config value drops out as it isn't supported server side | ||
|
There was a problem hiding this comment. "config value" refers to c.TemplateNamespaces ? There was a problem hiding this comment.
tempateinformers There was a problem hiding this comment. so where does the template informer get its config from at that point? and is that something one of your other pulls changes? There was a problem hiding this comment.
5 lines down in this pull. There was a problem hiding this comment. i'm lost. In the case where this code is running in a pod (not in the master), you're currently standing up an informer using a client that's created using the in cluster config. the comment says "eventually the config value drops out". What is meant by eventually, are you referring to a future evolution of this code(my original interpretation), or are you referring to some downstream codepath that dumps the value? And if the latter, what is meant by "drops out"? There was a problem hiding this comment.
Future evolution removes this code. See e27e5d5#diff-e73d057d1e253024c42b0fec37e12420L59 |
||
| inCluster = true | ||
| clientConfig, err := restclient.InClusterConfig() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| templateClient, err := templateinternalclientset.NewForConfig(clientConfig) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| templateInformers := templateinformer.NewSharedInformerFactory(templateClient, 5*time.Minute) | ||
| templateInformers.Template().InternalVersion().Templates().Informer().AddIndexers(cache.Indexers{ | ||
| templateapi.TemplateUIDIndex: func(obj interface{}) ([]string, error) { | ||
| return []string{string(obj.(*templateapi.Template).UID)}, nil | ||
| }, | ||
| }) | ||
|
|
||
| broker, err = templateservicebroker.NewBroker( | ||
| clientConfig, | ||
| templateInformers.Template().InternalVersion().Templates(), | ||
| c.TemplateNamespaces, | ||
| ) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| s.GenericAPIServer.AddPostStartHook("template-service-broker-synctemplates", func(context genericapiserver.PostStartHookContext) error { | ||
| templateInformers.Start(context.StopCh) | ||
| if !controller.WaitForCacheSync("tsb", context.StopCh, templateInformers.Template().InternalVersion().Templates().Informer().HasSynced) { | ||
| return fmt.Errorf("unable to sync caches") | ||
| } | ||
| return nil | ||
| }) | ||
| } | ||
|
|
||
| Route( | ||
| s.GenericAPIServer.Handler.GoRestfulContainer, | ||
|
|
@@ -100,32 +147,34 @@ func (c completedTemplateServiceBrokerConfig) New(delegationTarget genericapiser | |
| ) | ||
|
|
||
| // TODO, when the TSB becomes a separate entity, this should stop creating the SA and use its container pod SA identity instead | ||
| if !inCluster { | ||
| s.GenericAPIServer.AddPostStartHook("template-service-broker-ensure-service-account", func(context genericapiserver.PostStartHookContext) error { | ||
| kc, err := kclientsetinternal.NewForConfig(context.LoopbackClientConfig) | ||
| if err != nil { | ||
| utilruntime.HandleError(fmt.Errorf("template service broker: failed to get client: %v", err)) | ||
| return err | ||
| } | ||
|
|
||
| err = wait.PollImmediate(time.Second, 30*time.Second, func() (done bool, err error) { | ||
| kc.Namespaces().Create(&api.Namespace{ObjectMeta: metav1.ObjectMeta{Name: bootstrappolicy.DefaultOpenShiftInfraNamespace}}) | ||
|
|
||
| _, err = kc.ServiceAccounts(bootstrappolicy.DefaultOpenShiftInfraNamespace).Create(&api.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: bootstrappolicy.InfraTemplateServiceBrokerServiceAccountName}}) | ||
| switch { | ||
| case err == nil || kapierrors.IsAlreadyExists(err): | ||
| done, err = true, nil | ||
| case kapierrors.IsNotFound(err): | ||
| err = nil | ||
| } | ||
|
|
||
| return | ||
| }) | ||
|
|
||
| if err != nil { | ||
| utilruntime.HandleError(fmt.Errorf("creation of template-service-broker SA failed: %v", err)) | ||
| } | ||
| return err | ||
| }) | ||
| } | ||
|
|
||
| return s, nil | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm guessing this is making our audit harder, of course. Although the advanced audit is designed to work with multiple api server I'm not 100% sure if that was tested ;) Btw. do we care about upstream audit options here. We haven't decided yet which parts of the advanced audit we will enable and this is literally taking what upstream has.