Skip to content

Commit 9a9d413

Browse files
openshift-merge-robotopenshift-merge-robot
authored
Merge pull request #16533 from deads2k/deploy-01-bad-index
Automatic merge from submit-queue (batch tested with PRs 16534, 16533, 16510, 16508, 16392) remove legacy dc cache methods Switches the DC controllers to shared informers and listers instead of their one offs. @mfojtik this is part of what is under `pkg/client`
2 parents 4be0f20 + dfd7437 commit 9a9d413

File tree

12 files changed

+238
-311
lines changed

12 files changed

+238
-311
lines changed

‎pkg/apps/controller/deploymentconfig/deploymentconfig_controller.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ import (
2323

2424
deployapi "github.com/openshift/origin/pkg/apps/apis/apps"
2525
appsclient "github.com/openshift/origin/pkg/apps/generated/internalclientset/typed/apps/internalversion"
26+
appslister "github.com/openshift/origin/pkg/apps/generated/listers/apps/internalversion"
2627
deployutil "github.com/openshift/origin/pkg/apps/util"
27-
oscache "github.com/openshift/origin/pkg/client/cache"
2828
)
2929

3030
const (
@@ -63,8 +63,8 @@ type DeploymentConfigController struct {
6363
// queue contains deployment configs that need to be synced.
6464
queue workqueue.RateLimitingInterface
6565

66-
// dcStore provides a local cache for deployment configs.
67-
dcStore oscache.StoreToDeploymentConfigLister
66+
// dcLister provides a local cache for deployment configs.
67+
dcLister appslister.DeploymentConfigLister
6868
// dcStoreSynced makes sure the dc store is synced before reconcling any deployment config.
6969
dcStoreSynced func() bool
7070
// rcLister can list/get replication controllers from a shared informer's cache

‎pkg/apps/controller/deploymentconfig/deploymentconfig_controller_test.go

+27-12
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
deploytest "github.com/openshift/origin/pkg/apps/apis/apps/test"
2626
deployv1 "github.com/openshift/origin/pkg/apps/apis/apps/v1"
2727
appsfake "github.com/openshift/origin/pkg/apps/generated/internalclientset/fake"
28+
"github.com/openshift/origin/pkg/apps/generated/listers/apps/internalversion"
2829
deployutil "github.com/openshift/origin/pkg/apps/util"
2930
)
3031

@@ -366,19 +367,21 @@ func TestHandleScenarios(t *testing.T) {
366367
})
367368
codec := kapi.Codecs.LegacyCodec(deployv1.SchemeGroupVersion)
368369

369-
dcInformer := cache.NewSharedIndexInformer(
370-
&cache.ListWatch{
371-
ListFunc: func(options metav1.ListOptions) (runtime.Object, error) {
372-
return oc.Apps().DeploymentConfigs(metav1.NamespaceAll).List(options)
370+
dcInformer := &fakeDeploymentConfigInformer{
371+
informer: cache.NewSharedIndexInformer(
372+
&cache.ListWatch{
373+
ListFunc: func(options metav1.ListOptions) (runtime.Object, error) {
374+
return oc.Apps().DeploymentConfigs(metav1.NamespaceAll).List(options)
375+
},
376+
WatchFunc: func(options metav1.ListOptions) (watch.Interface, error) {
377+
return oc.Apps().DeploymentConfigs(metav1.NamespaceAll).Watch(options)
378+
},
373379
},
374-
WatchFunc: func(options metav1.ListOptions) (watch.Interface, error) {
375-
return oc.Apps().DeploymentConfigs(metav1.NamespaceAll).Watch(options)
376-
},
377-
},
378-
&deployapi.DeploymentConfig{},
379-
2*time.Minute,
380-
cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc},
381-
)
380+
&deployapi.DeploymentConfig{},
381+
2*time.Minute,
382+
cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc},
383+
),
384+
}
382385

383386
kubeInformerFactory := kinformers.NewSharedInformerFactory(kc, 0)
384387
rcInformer := kubeInformerFactory.Core().V1().ReplicationControllers()
@@ -430,6 +433,18 @@ func TestHandleScenarios(t *testing.T) {
430433
}
431434
}
432435

436+
type fakeDeploymentConfigInformer struct {
437+
informer cache.SharedIndexInformer
438+
}
439+
440+
func (f *fakeDeploymentConfigInformer) Informer() cache.SharedIndexInformer {
441+
return f.informer
442+
}
443+
444+
func (f *fakeDeploymentConfigInformer) Lister() internalversion.DeploymentConfigLister {
445+
return internalversion.NewDeploymentConfigLister(f.informer.GetIndexer())
446+
}
447+
433448
func newInt32(i int32) *int32 {
434449
return &i
435450
}

‎pkg/apps/controller/deploymentconfig/factory.go

+16-23
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"time"
66

77
"github.com/golang/glog"
8+
"k8s.io/apimachinery/pkg/api/errors"
89
"k8s.io/apimachinery/pkg/runtime"
910
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
1011
"k8s.io/apimachinery/pkg/util/wait"
@@ -20,6 +21,7 @@ import (
2021
kcontroller "k8s.io/kubernetes/pkg/controller"
2122

2223
deployapi "github.com/openshift/origin/pkg/apps/apis/apps"
24+
appsinformer "github.com/openshift/origin/pkg/apps/generated/informers/internalversion/apps/internalversion"
2325
appsclient "github.com/openshift/origin/pkg/apps/generated/internalclientset"
2426
metrics "github.com/openshift/origin/pkg/apps/metrics/prometheus"
2527
)
@@ -33,7 +35,7 @@ const (
3335

3436
// NewDeploymentConfigController creates a new DeploymentConfigController.
3537
func NewDeploymentConfigController(
36-
dcInformer cache.SharedIndexInformer,
38+
dcInformer appsinformer.DeploymentConfigInformer,
3739
rcInformer kcoreinformers.ReplicationControllerInformer,
3840
appsClientset appsclient.Interface,
3941
kubeClientset kclientset.Interface,
@@ -60,13 +62,13 @@ func NewDeploymentConfigController(
6062
codec: codec,
6163
}
6264

63-
c.dcStore.Indexer = dcInformer.GetIndexer()
64-
dcInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{
65+
c.dcLister = dcInformer.Lister()
66+
dcInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
6567
AddFunc: c.addDeploymentConfig,
6668
UpdateFunc: c.updateDeploymentConfig,
6769
DeleteFunc: c.deleteDeploymentConfig,
6870
})
69-
c.dcStoreSynced = dcInformer.HasSynced
71+
c.dcStoreSynced = dcInformer.Informer().HasSynced
7072

7173
rcInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
7274
UpdateFunc: c.updateReplicationController,
@@ -147,7 +149,7 @@ func (c *DeploymentConfigController) updateReplicationController(old, cur interf
147149
return
148150
}
149151

150-
if dc, err := c.dcStore.GetConfigForController(curRC); err == nil && dc != nil {
152+
if dc, err := c.dcLister.GetConfigForController(curRC); err == nil && dc != nil {
151153
c.enqueueDeploymentConfig(dc)
152154
}
153155
}
@@ -173,7 +175,7 @@ func (c *DeploymentConfigController) deleteReplicationController(obj interface{}
173175
return
174176
}
175177
}
176-
if dc, err := c.dcStore.GetConfigForController(rc); err == nil && dc != nil {
178+
if dc, err := c.dcLister.GetConfigForController(rc); err == nil && dc != nil {
177179
c.enqueueDeploymentConfig(dc)
178180
}
179181
}
@@ -202,12 +204,17 @@ func (c *DeploymentConfigController) work() bool {
202204
}
203205
defer c.queue.Done(key)
204206

205-
dc, err := c.getByKey(key.(string))
207+
namespace, name, err := cache.SplitMetaNamespaceKey(key.(string))
206208
if err != nil {
207209
utilruntime.HandleError(err)
210+
return false
208211
}
209-
210-
if dc == nil {
212+
dc, err := c.dcLister.DeploymentConfigs(namespace).Get(name)
213+
if errors.IsNotFound(err) {
214+
return false
215+
}
216+
if err != nil {
217+
utilruntime.HandleError(err)
211218
return false
212219
}
213220

@@ -216,17 +223,3 @@ func (c *DeploymentConfigController) work() bool {
216223

217224
return false
218225
}
219-
220-
func (c *DeploymentConfigController) getByKey(key string) (*deployapi.DeploymentConfig, error) {
221-
obj, exists, err := c.dcStore.Indexer.GetByKey(key)
222-
if err != nil {
223-
c.queue.AddRateLimited(key)
224-
return nil, err
225-
}
226-
if !exists {
227-
glog.V(4).Infof("Deployment config %q has been deleted", key)
228-
return nil, nil
229-
}
230-
231-
return obj.(*deployapi.DeploymentConfig), nil
232-
}

‎pkg/apps/controller/generictrigger/deploy_generictrigger_controller.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import (
99
"github.com/golang/glog"
1010
deployapi "github.com/openshift/origin/pkg/apps/apis/apps"
1111
appsclient "github.com/openshift/origin/pkg/apps/generated/internalclientset/typed/apps/internalversion"
12-
oscache "github.com/openshift/origin/pkg/client/cache"
12+
appslister "github.com/openshift/origin/pkg/apps/generated/listers/apps/internalversion"
1313
)
1414

1515
const (
@@ -32,7 +32,7 @@ type DeploymentTriggerController struct {
3232
queue workqueue.RateLimitingInterface
3333

3434
// dcLister provides a local cache for deployment configs.
35-
dcLister oscache.StoreToDeploymentConfigLister
35+
dcLister appslister.DeploymentConfigLister
3636
// dcListerSynced makes sure the dc store is synced before reconcling any deployment config.
3737
dcListerSynced func() bool
3838
// rcLister provides a local cache for replication controllers.

‎pkg/apps/controller/generictrigger/deploy_generictrigger_controller_test.go

+78-36
Original file line numberDiff line numberDiff line change
@@ -11,59 +11,101 @@ import (
1111
"k8s.io/client-go/tools/cache"
1212
kapi "k8s.io/kubernetes/pkg/api"
1313
"k8s.io/kubernetes/pkg/client/clientset_generated/clientset/fake"
14+
corelister "k8s.io/kubernetes/pkg/client/listers/core/v1"
1415

1516
deployapi "github.com/openshift/origin/pkg/apps/apis/apps"
1617
_ "github.com/openshift/origin/pkg/apps/apis/apps/install"
1718
testapi "github.com/openshift/origin/pkg/apps/apis/apps/test"
1819
deployv1 "github.com/openshift/origin/pkg/apps/apis/apps/v1"
1920
appsfake "github.com/openshift/origin/pkg/apps/generated/internalclientset/fake"
21+
appslister "github.com/openshift/origin/pkg/apps/generated/listers/apps/internalversion"
2022
imageapi "github.com/openshift/origin/pkg/image/apis/image"
2123
imagefake "github.com/openshift/origin/pkg/image/generated/internalclientset/fake"
24+
imagelister "github.com/openshift/origin/pkg/image/generated/listers/image/internalversion"
2225
)
2326

2427
var (
2528
codec = kapi.Codecs.LegacyCodec(deployv1.SchemeGroupVersion)
26-
dcInformer = cache.NewSharedIndexInformer(
27-
&cache.ListWatch{
28-
ListFunc: func(options metav1.ListOptions) (runtime.Object, error) {
29-
return (appsfake.NewSimpleClientset()).Apps().DeploymentConfigs(metav1.NamespaceAll).List(options)
29+
dcInformer = &fakeDeploymentConfigInformer{
30+
informer: cache.NewSharedIndexInformer(
31+
&cache.ListWatch{
32+
ListFunc: func(options metav1.ListOptions) (runtime.Object, error) {
33+
return (appsfake.NewSimpleClientset()).Apps().DeploymentConfigs(metav1.NamespaceAll).List(options)
34+
},
35+
WatchFunc: func(options metav1.ListOptions) (watch.Interface, error) {
36+
return (appsfake.NewSimpleClientset()).Apps().DeploymentConfigs(metav1.NamespaceAll).Watch(options)
37+
},
3038
},
31-
WatchFunc: func(options metav1.ListOptions) (watch.Interface, error) {
32-
return (appsfake.NewSimpleClientset()).Apps().DeploymentConfigs(metav1.NamespaceAll).Watch(options)
33-
},
34-
},
35-
&deployapi.DeploymentConfig{},
36-
2*time.Minute,
37-
cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc},
38-
)
39-
rcInformer = cache.NewSharedIndexInformer(
40-
&cache.ListWatch{
41-
ListFunc: func(options metav1.ListOptions) (runtime.Object, error) {
42-
return (fake.NewSimpleClientset()).Core().ReplicationControllers(metav1.NamespaceAll).List(options)
43-
},
44-
WatchFunc: func(options metav1.ListOptions) (watch.Interface, error) {
45-
return (fake.NewSimpleClientset()).Core().ReplicationControllers(metav1.NamespaceAll).Watch(options)
46-
},
47-
},
48-
&kapi.ReplicationController{},
49-
2*time.Minute,
50-
cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc},
51-
)
52-
streamInformer = cache.NewSharedIndexInformer(
53-
&cache.ListWatch{
54-
ListFunc: func(options metav1.ListOptions) (runtime.Object, error) {
55-
return (imagefake.NewSimpleClientset()).Image().ImageStreams(metav1.NamespaceAll).List(options)
39+
&deployapi.DeploymentConfig{},
40+
2*time.Minute,
41+
cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc},
42+
),
43+
}
44+
rcInformer = &fakeReplicationControllerInformer{
45+
informer: cache.NewSharedIndexInformer(
46+
&cache.ListWatch{
47+
ListFunc: func(options metav1.ListOptions) (runtime.Object, error) {
48+
return (fake.NewSimpleClientset()).Core().ReplicationControllers(metav1.NamespaceAll).List(options)
49+
},
50+
WatchFunc: func(options metav1.ListOptions) (watch.Interface, error) {
51+
return (fake.NewSimpleClientset()).Core().ReplicationControllers(metav1.NamespaceAll).Watch(options)
52+
},
5653
},
57-
WatchFunc: func(options metav1.ListOptions) (watch.Interface, error) {
58-
return (imagefake.NewSimpleClientset()).Image().ImageStreams(metav1.NamespaceAll).Watch(options)
54+
&kapi.ReplicationController{},
55+
2*time.Minute,
56+
cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc},
57+
),
58+
}
59+
streamInformer = &fakeImageStreamInformer{
60+
informer: cache.NewSharedIndexInformer(
61+
&cache.ListWatch{
62+
ListFunc: func(options metav1.ListOptions) (runtime.Object, error) {
63+
return (imagefake.NewSimpleClientset()).Image().ImageStreams(metav1.NamespaceAll).List(options)
64+
},
65+
WatchFunc: func(options metav1.ListOptions) (watch.Interface, error) {
66+
return (imagefake.NewSimpleClientset()).Image().ImageStreams(metav1.NamespaceAll).Watch(options)
67+
},
5968
},
60-
},
61-
&imageapi.ImageStream{},
62-
2*time.Minute,
63-
cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc},
64-
)
69+
&imageapi.ImageStream{},
70+
2*time.Minute,
71+
cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc},
72+
),
73+
}
6574
)
6675

76+
type fakeDeploymentConfigInformer struct {
77+
informer cache.SharedIndexInformer
78+
}
79+
80+
func (f *fakeDeploymentConfigInformer) Informer() cache.SharedIndexInformer {
81+
return f.informer
82+
}
83+
func (f *fakeDeploymentConfigInformer) Lister() appslister.DeploymentConfigLister {
84+
return appslister.NewDeploymentConfigLister(f.informer.GetIndexer())
85+
}
86+
87+
type fakeReplicationControllerInformer struct {
88+
informer cache.SharedIndexInformer
89+
}
90+
91+
func (f *fakeReplicationControllerInformer) Informer() cache.SharedIndexInformer {
92+
return f.informer
93+
}
94+
func (f *fakeReplicationControllerInformer) Lister() corelister.ReplicationControllerLister {
95+
return corelister.NewReplicationControllerLister(f.informer.GetIndexer())
96+
}
97+
98+
type fakeImageStreamInformer struct {
99+
informer cache.SharedIndexInformer
100+
}
101+
102+
func (f *fakeImageStreamInformer) Informer() cache.SharedIndexInformer {
103+
return f.informer
104+
}
105+
func (f *fakeImageStreamInformer) Lister() imagelister.ImageStreamLister {
106+
return imagelister.NewImageStreamLister(f.informer.GetIndexer())
107+
}
108+
67109
// TestHandle_noTriggers ensures that a change to a config with no
68110
// triggers doesn't result in a config instantiation.
69111
func TestHandle_noTriggers(t *testing.T) {

0 commit comments

Comments
 (0)