Skip to content

Commit bac08ba

Browse files
Merge pull request #1863 from vrutkovs/goaway-chance-0-001
OCPBUGS-43521: Set goaway chance to 0.001
2 parents aef41d7 + 6ea3bb4 commit bac08ba

File tree

5 files changed

+235
-1
lines changed

5 files changed

+235
-1
lines changed

bindata/assets/config/defaultconfig.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ apiServerArguments:
113113
event-ttl:
114114
- 3h
115115
goaway-chance:
116-
- "0"
116+
- "0.001"
117117
http2-max-streams-per-connection:
118118
- "2000" # recommended is 1000, but we need to mitigate https://github.com/kubernetes/kubernetes/issues/74412
119119
kubelet-certificate-authority:

pkg/cmd/render/render.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,9 @@ type TemplateData struct {
195195
// ShutdownDelayDuration is passed to kube-apiserver. Empty means not to override defaultconfig's value.
196196
ShutdownDelayDuration string
197197

198+
// GoAwayChance is defaultconfig's goaway-chance setting. Empty means not to override defaultconfig's value.
199+
GoAwayChance string
200+
198201
ServiceAccountIssuer string
199202
}
200203

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
package apiserver
2+
3+
import (
4+
"fmt"
5+
6+
apierrors "k8s.io/apimachinery/pkg/api/errors"
7+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
8+
"k8s.io/klog/v2"
9+
10+
configv1 "github.com/openshift/api/config/v1"
11+
"github.com/openshift/cluster-kube-apiserver-operator/pkg/operator/configobservation"
12+
"github.com/openshift/library-go/pkg/operator/configobserver"
13+
"github.com/openshift/library-go/pkg/operator/events"
14+
)
15+
16+
var goawayChancePath = []string{"apiServerArguments", "goaway-chance"}
17+
18+
// ObserveGoawayChance ensures that goaway-chance is 0 for SNO topology
19+
func ObserveGoawayChance(genericListers configobserver.Listers, _ events.Recorder, existingConfig map[string]interface{}) (ret map[string]interface{}, errs []error) {
20+
defer func() {
21+
// Prune the observed config so that it only contains apiServerArguments field.
22+
ret = configobserver.Pruned(ret, goawayChancePath)
23+
}()
24+
25+
// read the observed value
26+
listers := genericListers.(configobservation.Listers)
27+
infra, err := listers.InfrastructureLister().Get("cluster")
28+
if err != nil {
29+
// we got an error so without the infrastructure object we are not able to determine the type of platform we are running on
30+
if apierrors.IsNotFound(err) {
31+
klog.Warningf("ObserveGoawayChance: infras.%s/cluster not found", configv1.GroupName)
32+
} else {
33+
errs = append(errs, err)
34+
}
35+
return existingConfig, errs
36+
}
37+
38+
observedGoawayChance := "0.001"
39+
if infra.Status.ControlPlaneTopology == configv1.SingleReplicaTopologyMode {
40+
// for SNO we want to set goaway-chance to 0
41+
observedGoawayChance = "0"
42+
}
43+
44+
// read the current value
45+
var currentGoawayChance string
46+
currentGoawayChanceSlice, _, err := unstructured.NestedStringSlice(existingConfig, goawayChancePath...)
47+
if err != nil {
48+
errs = append(errs, fmt.Errorf("unable to extract goaway chance setting from the existing config: %v", err))
49+
// keep going, we are only interested in the observed value which will overwrite the current configuration anyway
50+
}
51+
if len(currentGoawayChanceSlice) > 0 {
52+
currentGoawayChance = currentGoawayChanceSlice[0]
53+
}
54+
55+
// see if the current and the observed value differ
56+
observedConfig := map[string]interface{}{}
57+
if observedGoawayChance != currentGoawayChance {
58+
if err = unstructured.SetNestedStringSlice(observedConfig, []string{observedGoawayChance}, goawayChancePath...); err != nil {
59+
return existingConfig, append(errs, err)
60+
}
61+
return observedConfig, errs
62+
}
63+
64+
// nothing has changed return the original configuration
65+
return existingConfig, errs
66+
}
Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
package apiserver
2+
3+
import (
4+
"testing"
5+
6+
"github.com/google/go-cmp/cmp"
7+
8+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
9+
"k8s.io/client-go/tools/cache"
10+
"k8s.io/utils/clock"
11+
12+
configv1 "github.com/openshift/api/config/v1"
13+
configlistersv1 "github.com/openshift/client-go/config/listers/config/v1"
14+
"github.com/openshift/cluster-kube-apiserver-operator/pkg/operator/configobservation"
15+
"github.com/openshift/library-go/pkg/operator/events"
16+
)
17+
18+
func TestObserveGoawayChance(t *testing.T) {
19+
scenarios := []struct {
20+
name string
21+
existingKubeAPIConfig map[string]interface{}
22+
expectedKubeAPIConfig map[string]interface{}
23+
controlPlaneTopology configv1.TopologyMode
24+
}{
25+
26+
// scenario 1 - HA unset
27+
{
28+
name: "ha: defaults to 0.001",
29+
controlPlaneTopology: configv1.HighlyAvailableTopologyMode,
30+
expectedKubeAPIConfig: map[string]interface{}{"apiServerArguments": map[string]interface{}{
31+
"goaway-chance": []interface{}{"0.001"},
32+
}},
33+
},
34+
35+
// scenario 3 - HA, update not required
36+
{
37+
name: "ha: update not required",
38+
controlPlaneTopology: configv1.HighlyAvailableTopologyMode,
39+
existingKubeAPIConfig: map[string]interface{}{"apiServerArguments": map[string]interface{}{
40+
"goaway-chance": []interface{}{"0.001"},
41+
}},
42+
expectedKubeAPIConfig: map[string]interface{}{"apiServerArguments": map[string]interface{}{
43+
"goaway-chance": []interface{}{"0.001"},
44+
}},
45+
},
46+
47+
// scenario 4 - HA, update required
48+
{
49+
name: "ha: update required",
50+
controlPlaneTopology: configv1.HighlyAvailableTopologyMode,
51+
existingKubeAPIConfig: map[string]interface{}{"apiServerArguments": map[string]interface{}{
52+
"goaway-chance": []interface{}{"0.2"},
53+
}},
54+
expectedKubeAPIConfig: map[string]interface{}{"apiServerArguments": map[string]interface{}{
55+
"goaway-chance": []interface{}{"0.001"},
56+
}},
57+
},
58+
59+
// scenario 5 - SNO
60+
{
61+
name: "sno: defaults to 0",
62+
controlPlaneTopology: configv1.SingleReplicaTopologyMode,
63+
expectedKubeAPIConfig: map[string]interface{}{"apiServerArguments": map[string]interface{}{
64+
"goaway-chance": []interface{}{"0"},
65+
}},
66+
},
67+
68+
// scenario 6 - SNO, update required
69+
{
70+
name: "sno: update required",
71+
controlPlaneTopology: configv1.SingleReplicaTopologyMode,
72+
existingKubeAPIConfig: map[string]interface{}{"apiServerArguments": map[string]interface{}{
73+
"goaway-chance": []interface{}{"0.001"},
74+
}},
75+
expectedKubeAPIConfig: map[string]interface{}{"apiServerArguments": map[string]interface{}{
76+
"goaway-chance": []interface{}{"0"},
77+
}},
78+
},
79+
80+
// scenario 7 - SNO, update not required
81+
{
82+
name: "sno: update not required",
83+
controlPlaneTopology: configv1.SingleReplicaTopologyMode,
84+
existingKubeAPIConfig: map[string]interface{}{"apiServerArguments": map[string]interface{}{
85+
"goaway-chance": []interface{}{"0"},
86+
}},
87+
expectedKubeAPIConfig: map[string]interface{}{"apiServerArguments": map[string]interface{}{
88+
"goaway-chance": []interface{}{"0"},
89+
}},
90+
},
91+
}
92+
93+
for _, scenario := range scenarios {
94+
t.Run(scenario.name, func(t *testing.T) {
95+
// test data
96+
eventRecorder := events.NewInMemoryRecorder("", clock.RealClock{})
97+
infrastructureIndexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{})
98+
infrastructureIndexer.Add(&configv1.Infrastructure{
99+
ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
100+
Status: configv1.InfrastructureStatus{ControlPlaneTopology: scenario.controlPlaneTopology},
101+
})
102+
listers := configobservation.Listers{
103+
InfrastructureLister_: configlistersv1.NewInfrastructureLister(infrastructureIndexer),
104+
}
105+
106+
// act
107+
observedKubeAPIConfig, err := ObserveGoawayChance(listers, eventRecorder, scenario.existingKubeAPIConfig)
108+
109+
// validate
110+
if len(err) > 0 {
111+
t.Fatal(err)
112+
}
113+
if diff := cmp.Diff(scenario.expectedKubeAPIConfig, observedKubeAPIConfig); diff != "" {
114+
t.Fatalf("unexpected configuration, diff = %s", diff)
115+
}
116+
})
117+
}
118+
}
119+
120+
func TestObserveGoawayChanceErrors(t *testing.T) {
121+
scenarios := []struct {
122+
name string
123+
infraIndexerFunc func(cache.Indexer)
124+
expectedErrs []error
125+
}{
126+
{
127+
name: "happy path",
128+
infraIndexerFunc: func(indexer cache.Indexer) {
129+
indexer.Add(&configv1.Infrastructure{
130+
ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
131+
Status: configv1.InfrastructureStatus{ControlPlaneTopology: configv1.HighlyAvailableTopologyMode},
132+
})
133+
},
134+
expectedErrs: nil,
135+
},
136+
{
137+
name: "no cluster infra",
138+
infraIndexerFunc: nil,
139+
expectedErrs: nil,
140+
},
141+
}
142+
143+
for _, scenario := range scenarios {
144+
t.Run(scenario.name, func(t *testing.T) {
145+
// test data
146+
eventRecorder := events.NewInMemoryRecorder("", clock.RealClock{})
147+
infrastructureIndexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{})
148+
if scenario.infraIndexerFunc != nil {
149+
scenario.infraIndexerFunc(infrastructureIndexer)
150+
}
151+
listers := configobservation.Listers{
152+
InfrastructureLister_: configlistersv1.NewInfrastructureLister(infrastructureIndexer),
153+
}
154+
155+
// act
156+
_, errs := ObserveGoawayChance(listers, eventRecorder, map[string]interface{}{})
157+
158+
// validate
159+
if diff := cmp.Diff(scenario.expectedErrs, errs); diff != "" {
160+
t.Fatalf("unexpected errs, diff = %s", diff)
161+
}
162+
})
163+
}
164+
}

pkg/operator/configobservation/configobservercontroller/observe_config_controller.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ func NewConfigObserver(operatorClient v1helpers.StaticPodOperatorClient, kubeInf
123123
apiserver.ObserveShutdownDelayDuration,
124124
apiserver.ObserveGracefulTerminationDuration,
125125
apiserver.ObserveSendRetryAfterWhileNotReadyOnce,
126+
apiserver.ObserveGoawayChance,
126127
apiserver.ObserveAdmissionPlugins,
127128
libgoapiserver.ObserveTLSSecurityProfile,
128129
auth.ObserveAuthMetadata,

0 commit comments

Comments
 (0)