Skip to content

Commit 91fcf31

Browse files
jcmoraisjrjcmoraisjr
committed
Select HAProxy version from IngressController API
Configures the HAProxy sidecar with the image URL representing the HAProxy version configured in the IngressController API. image-references manifest updated with the new HAProxy images, their image names added in the deployment manifest, which configures new command-line options with the new HAProxy image URLs. The haproxyVersion reading is under the FeatureGateIngressControllerMultipleHAProxyVersions feature gate, and the router image, which also has HAProxy installed, is used if the feature gate is disabled. https://redhat.atlassian.net/browse/NE-2219
1 parent 5b910f7 commit 91fcf31

11 files changed

Lines changed: 202 additions & 4 deletions

File tree

‎cmd/ingress-operator/start.go‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
corev1 "k8s.io/api/core/v1"
2727
"k8s.io/apimachinery/pkg/types"
2828

29+
operatorv1 "github.com/openshift/api/operator/v1"
2930
unidlingapi "github.com/openshift/api/unidling/v1alpha1"
3031
)
3132

@@ -51,6 +52,10 @@ type StartOptions struct {
5152
// IngressControllerImage is the pullspec of the ingress controller image to
5253
// be managed.
5354
IngressControllerImage string
55+
// HAProxyImages is the pullspec of the HAProxy images to be managed, as a hashmap of `<version>:<pullspec>`.
56+
HAProxyImages map[string]string
57+
// DefaultHAProxyVersion is the default HAProxy version.
58+
DefaultHAProxyVersion string
5459
// CanaryImage is the pullspec of the ingress operator image
5560
CanaryImage string
5661
// ReleaseVersion is the cluster version which the operator will converge to.
@@ -82,6 +87,8 @@ func NewStartCommand() *cobra.Command {
8287

8388
cmd.Flags().StringVarP(&options.OperatorNamespace, "namespace", "n", operatorcontroller.DefaultOperatorNamespace, "namespace the operator is deployed to (required)")
8489
cmd.Flags().StringVarP(&options.IngressControllerImage, "image", "i", "", "image of the ingress controller the operator will manage (required)")
90+
cmd.Flags().StringToStringVarP(&options.HAProxyImages, "haproxy-image", "", nil, "HAProxy images as version=pullspec (optional)")
91+
cmd.Flags().StringVarP(&options.DefaultHAProxyVersion, "default-haproxy-version", "", "", "defines the default HAProxy version (optional)")
8592
cmd.Flags().StringVarP(&options.CanaryImage, "canary-image", "c", "", "image of the canary container that the operator will manage (optional)")
8693
cmd.Flags().StringVarP(&options.ReleaseVersion, "release-version", "", statuscontroller.UnknownVersionValue, "the release version the operator should converge to (required)")
8794
cmd.Flags().StringVarP(&options.MetricsListenAddr, "metrics-listen-addr", "", "127.0.0.1:60000", "metrics endpoint listen address (required)")
@@ -133,10 +140,21 @@ func start(opts *StartOptions) error {
133140
signal, cancel := context.WithCancel(signals.SetupSignalHandler())
134141
defer cancel()
135142

143+
// Convert the --haproxy-image input into the haproxyImages hashmap.
144+
haproxyImages := make(map[operatorv1.HAProxyVersion]string)
145+
for version, image := range opts.HAProxyImages {
146+
if version == "" || image == "" {
147+
return fmt.Errorf("invalid HAProxy version=pullspec: '%s=%s'", version, image)
148+
}
149+
haproxyImages[operatorv1.HAProxyVersion(version)] = image
150+
}
151+
136152
operatorConfig := operatorconfig.Config{
137153
OperatorReleaseVersion: opts.ReleaseVersion,
138154
Namespace: opts.OperatorNamespace,
139155
IngressControllerImage: opts.IngressControllerImage,
156+
HAProxyImages: haproxyImages,
157+
DefaultHAProxyVersion: operatorv1.HAProxyVersion(opts.DefaultHAProxyVersion),
140158
CanaryImage: opts.CanaryImage,
141159
GatewayAPIOperatorCatalog: opts.GatewayAPIOperatorCatalog,
142160
GatewayAPIOperatorChannel: opts.GatewayAPIOperatorChannel,

‎manifests/02-deployment-ibm-cloud-managed.yaml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,10 @@ spec:
3131
- $(WATCH_NAMESPACE)
3232
- --image
3333
- $(IMAGE)
34+
- --haproxy-image
35+
- 2.8=$(HAPROXY_28_IMAGE)
36+
- --haproxy-image
37+
- 3.2=$(HAPROXY_32_IMAGE)
3438
- --canary-image
3539
- $(CANARY_IMAGE)
3640
- --release-version
@@ -52,6 +56,10 @@ spec:
5256
fieldPath: metadata.namespace
5357
- name: IMAGE
5458
value: openshift/origin-haproxy-router:v4.0
59+
- name: HAPROXY_28_IMAGE
60+
value: openshift/origin-haproxy:v2.8
61+
- name: HAPROXY_32_IMAGE
62+
value: openshift/origin-haproxy:v3.2
5563
- name: CANARY_IMAGE
5664
value: openshift/origin-cluster-ingress-operator:latest
5765
- name: GATEWAY_API_OPERATOR_CATALOG

‎manifests/02-deployment.yaml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,10 @@ spec:
6060
- "$(WATCH_NAMESPACE)"
6161
- --image
6262
- "$(IMAGE)"
63+
- --haproxy-image
64+
- "2.8=$(HAPROXY_28_IMAGE)"
65+
- --haproxy-image
66+
- "3.2=$(HAPROXY_32_IMAGE)"
6367
- --canary-image
6468
- "$(CANARY_IMAGE)"
6569
- --release-version
@@ -81,6 +85,10 @@ spec:
8185
fieldPath: metadata.namespace
8286
- name: IMAGE
8387
value: openshift/origin-haproxy-router:v4.0
88+
- name: HAPROXY_28_IMAGE
89+
value: openshift/origin-haproxy:v2.8
90+
- name: HAPROXY_32_IMAGE
91+
value: openshift/origin-haproxy:v3.2
8492
- name: CANARY_IMAGE
8593
value: openshift/origin-cluster-ingress-operator:latest
8694
- name: GATEWAY_API_OPERATOR_CATALOG

‎manifests/image-references‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,14 @@ spec:
1010
from:
1111
kind: "DockerImage"
1212
name: "openshift/origin-haproxy-router:v4.0"
13+
- name: haproxy-router-haproxy28
14+
from:
15+
kind: "DockerImage"
16+
name: "openshift/origin-haproxy:v2.8"
17+
- name: haproxy-router-haproxy32
18+
from:
19+
kind: "DockerImage"
20+
name: "openshift/origin-haproxy:v3.2"
1321
- name: kube-rbac-proxy
1422
from:
1523
kind: DockerImage

‎pkg/operator/config/config.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
package config
22

3+
import (
4+
operatorv1 "github.com/openshift/api/operator/v1"
5+
)
6+
37
// Config is configuration for the operator and should include things like
48
// operated images, scheduling configuration, etc.
59
type Config struct {
@@ -12,6 +16,12 @@ type Config struct {
1216
// IngressControllerImage is the ingress controller image to manage.
1317
IngressControllerImage string
1418

19+
// HAProxyImages is the map of HAProxy images to be managed.
20+
HAProxyImages map[operatorv1.HAProxyVersion]string
21+
22+
// DefaultHAProxyVersion defines the default HAProxy version.
23+
DefaultHAProxyVersion operatorv1.HAProxyVersion
24+
1525
// CanaryImage is the ingress operator image, which runs a canary command.
1626
CanaryImage string
1727

‎pkg/operator/controller/ingress/controller.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,10 @@ func hasName(name string) func(o client.Object) bool {
243243
type Config struct {
244244
Namespace string
245245
IngressControllerImage string
246+
HAProxyImages map[operatorv1.HAProxyVersion]string
247+
DefaultHAProxyVersion operatorv1.HAProxyVersion
246248
IngressControllerDCMEnabled bool
249+
FeatureMultiHAProxyEnabled bool
247250
}
248251

249252
// reconciler handles the actual ingress reconciliation logic in response to

‎pkg/operator/controller/ingress/deployment.go‎

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,10 @@ const (
165165
//
166166
// NOTE: it is hardcoded in the deployment.yaml manifest, update there if updating here.
167167
routerHAProxyAdminSocket = "/var/lib/haproxy/run/admin.sock"
168+
169+
// defaultHAProxyVersion defines the default HAProxy version, used in case
170+
// Config.DefaultHAProxyVersion is empty.
171+
defaultHAProxyVersion = operatorv1.HAProxyVersion32
168172
)
169173

170174
// ensureRouterDeployment ensures the router deployment exists for a given
@@ -830,10 +834,12 @@ func desiredRouterDeployment(ci *operatorv1.IngressController, config *Config, i
830834
// router image.
831835
deployment.Spec.Template.Spec.Containers[0].Image = config.IngressControllerImage
832836

833-
// haproxy image: using router's image for now, but it needs its own image.
834-
// HAProxy images: https://redhat.atlassian.net/browse/NE-2218
835-
// API field: https://redhat.atlassian.net/browse/NE-2217
836-
deployment.Spec.Template.Spec.InitContainers[1].Image = config.IngressControllerImage
837+
// haproxy image.
838+
_, haproxyImage, err := getEffectiveHAProxyVersionAndImage(ci, config)
839+
if err != nil {
840+
return nil, err
841+
}
842+
deployment.Spec.Template.Spec.InitContainers[1].Image = haproxyImage
837843

838844
deployment.Spec.Template.Spec.DNSPolicy = corev1.DNSClusterFirst
839845

@@ -1385,6 +1391,39 @@ func desiredRouterDeployment(ci *operatorv1.IngressController, config *Config, i
13851391
return deployment, nil
13861392
}
13871393

1394+
func getEffectiveHAProxyVersionAndImage(ic *operatorv1.IngressController, config *Config) (operatorv1.HAProxyVersion, string, error) {
1395+
if !config.FeatureMultiHAProxyEnabled {
1396+
// no effective version, and using router image in case FG is not enabled
1397+
return "", config.IngressControllerImage, nil
1398+
}
1399+
1400+
if len(config.HAProxyImages) == 0 {
1401+
// images map is currently optional, using router image as fallback
1402+
return operatorv1.HAProxyVersion28, config.IngressControllerImage, nil
1403+
}
1404+
1405+
// version is user provided, fallback to the default one if unset/empty/missing, default can be overridden via command-line option
1406+
version := ic.Spec.HAProxyVersion
1407+
if version == "" {
1408+
version = operatorv1.HAProxyVersion(config.DefaultHAProxyVersion)
1409+
if version == "" {
1410+
version = defaultHAProxyVersion
1411+
}
1412+
}
1413+
1414+
image, found := config.HAProxyImages[version]
1415+
if !found {
1416+
if ic.Spec.HAProxyVersion != "" {
1417+
// user provided an invalid one
1418+
return "", "", fmt.Errorf("ingresscontroller %q has invalid spec.haproxyVersion: %v", ic.Name, ic.Spec.HAProxyVersion)
1419+
}
1420+
// invalid configuration provided, either via the images hashmap or the overridden default
1421+
return "", "", fmt.Errorf("haproxy images map is missing the default version '%s': %+v", version, config.HAProxyImages)
1422+
}
1423+
1424+
return version, image, nil
1425+
}
1426+
13881427
// accessLoggingForIngressController returns an AccessLogging value for the
13891428
// given ingresscontroller, or nil if the ingresscontroller does not specify
13901429
// a valid access logging configuration.

‎pkg/operator/controller/ingress/deployment_test.go‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -899,6 +899,95 @@ func hasDesiredRouterDeploymentSpecTemplate(t *testing.T, ic *operatorv1.Ingress
899899
checkDeploymentHasEnvSorted(t, deployment)
900900
}
901901

902+
func TestDesiredRouterDeploymentSpecHAProxyVersion(t *testing.T) {
903+
var haproxyImages = map[operatorv1.HAProxyVersion]string{
904+
operatorv1.HAProxyVersion28: "quay.io/openshift/haproxy:2.8",
905+
operatorv1.HAProxyVersion32: "quay.io/openshift/haproxy:3.2",
906+
}
907+
908+
testCases := map[string]struct {
909+
featureGateEnabled bool
910+
haproxyImages map[operatorv1.HAProxyVersion]string
911+
defaultHAProxyVersion operatorv1.HAProxyVersion
912+
desiredHAProxyVersion operatorv1.HAProxyVersion
913+
expectedHAProxyImage string
914+
expectedError string
915+
}{
916+
"should deploy default haproxy if unset": {
917+
featureGateEnabled: true,
918+
haproxyImages: haproxyImages,
919+
defaultHAProxyVersion: defaultHAProxyVersion,
920+
desiredHAProxyVersion: "",
921+
expectedHAProxyImage: haproxyImages[defaultHAProxyVersion],
922+
},
923+
"should deploy default haproxy if unset and default is overridden": {
924+
featureGateEnabled: true,
925+
haproxyImages: haproxyImages,
926+
defaultHAProxyVersion: operatorv1.HAProxyVersion28,
927+
desiredHAProxyVersion: "",
928+
expectedHAProxyImage: haproxyImages[operatorv1.HAProxyVersion28],
929+
},
930+
"should deploy router image if images map is not provided": {
931+
featureGateEnabled: true,
932+
defaultHAProxyVersion: operatorv1.HAProxyVersion28,
933+
desiredHAProxyVersion: "",
934+
expectedHAProxyImage: ingressControllerImage,
935+
},
936+
"should deploy haproxy 3.2": {
937+
featureGateEnabled: true,
938+
haproxyImages: haproxyImages,
939+
defaultHAProxyVersion: defaultHAProxyVersion,
940+
desiredHAProxyVersion: operatorv1.HAProxyVersion32,
941+
expectedHAProxyImage: haproxyImages[operatorv1.HAProxyVersion32],
942+
},
943+
"should deploy haproxy 2.8": {
944+
featureGateEnabled: true,
945+
haproxyImages: haproxyImages,
946+
defaultHAProxyVersion: defaultHAProxyVersion,
947+
desiredHAProxyVersion: operatorv1.HAProxyVersion28,
948+
expectedHAProxyImage: haproxyImages[operatorv1.HAProxyVersion28],
949+
},
950+
"should fail on invalid haproxy version": {
951+
featureGateEnabled: true,
952+
haproxyImages: haproxyImages,
953+
defaultHAProxyVersion: defaultHAProxyVersion,
954+
desiredHAProxyVersion: "zz",
955+
expectedError: `ingresscontroller "default" has invalid spec.haproxyVersion: zz`,
956+
},
957+
"should ignore configuration if feature gate is disabled": {
958+
featureGateEnabled: false,
959+
haproxyImages: haproxyImages,
960+
defaultHAProxyVersion: defaultHAProxyVersion,
961+
desiredHAProxyVersion: "zz",
962+
expectedHAProxyImage: ingressControllerImage,
963+
},
964+
}
965+
966+
for name, test := range testCases {
967+
t.Run(name, func(t *testing.T) {
968+
ic, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, clusterProxyConfig := getRouterDeploymentComponents(t)
969+
ic.Spec.HAProxyVersion = test.desiredHAProxyVersion
970+
971+
deploymentConfig := &Config{
972+
IngressControllerImage: ingressControllerImage,
973+
HAProxyImages: test.haproxyImages,
974+
DefaultHAProxyVersion: test.defaultHAProxyVersion,
975+
FeatureMultiHAProxyEnabled: test.featureGateEnabled,
976+
}
977+
978+
deployment, err := desiredRouterDeployment(ic, deploymentConfig, ingressConfig, infraConfig, apiConfig, networkConfig, nil, proxyNeeded, false, nil, clusterProxyConfig)
979+
if test.expectedError == "" {
980+
require.NoError(t, err, "invalid router Deployment")
981+
initContainers := deployment.Spec.Template.Spec.InitContainers
982+
require.Len(t, initContainers, 2, "len(initContainers)")
983+
require.Equal(t, test.expectedHAProxyImage, initContainers[1].Image)
984+
} else {
985+
require.EqualError(t, err, test.expectedError)
986+
}
987+
})
988+
}
989+
}
990+
902991
func TestDesiredRouterDeploymentSpecAndNetwork(t *testing.T) {
903992
ic, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, clusterProxyConfig := getRouterDeploymentComponents(t)
904993

‎pkg/operator/controller/ingress/status.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,13 @@ func (r *reconciler) syncIngressControllerStatus(ic *operatorv1.IngressControlle
103103
updateIngressControllerFloatingIPOpenStackStatus(updated, service)
104104
}
105105

106+
haproxyVersion, _, err := getEffectiveHAProxyVersionAndImage(ic, &r.config)
107+
if err != nil {
108+
errs = append(errs, err)
109+
haproxyVersion = ""
110+
}
111+
updated.Status.EffectiveHAProxyVersion = haproxyVersion
112+
106113
updated.Status.Conditions = MergeConditions(updated.Status.Conditions, computeDeploymentAvailableCondition(deployment))
107114
updated.Status.Conditions = MergeConditions(updated.Status.Conditions, computeDeploymentReplicasMinAvailableCondition(deployment, pods))
108115
updated.Status.Conditions = MergeConditions(updated.Status.Conditions, computeDeploymentReplicasAllAvailableCondition(deployment))

‎pkg/operator/controller/status/controller.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,8 @@ type Config struct {
226226
OperatorLifecycleManagerEnabled bool
227227
GatewayAPIWithoutOLMEnabled bool
228228
IngressControllerImage string
229+
HAProxyImages map[operatorv1.HAProxyVersion]string
230+
DefaultHAProxyVersion operatorv1.HAProxyVersion
229231
CanaryImage string
230232
OperatorReleaseVersion string
231233
Namespace string

0 commit comments

Comments
 (0)