NE-2216: parameterize haproxy version#1362
NE-2216: parameterize haproxy version#1362jcmoraisjr wants to merge 1 commit intoopenshift:masterfrom
Conversation
add two new annotations used to parameterize the haproxy version to be used in the router. * haproxy-version: defines the haproxy version to use. I must be one of the installed versions in the default router image. If missing, it defaults to the older installed version. * haproxy-image: URL of the haproxy image to use. If configured, ingress operator deploys haproxy as a sidecar and configures it instead of the embedded haproxy from the router image.
|
@jcmoraisjr: This pull request references NE-2216 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@jcmoraisjr: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Thealisyed
left a comment
There was a problem hiding this comment.
It might also be worth noting that we should consider adding some test coverage for the annotations (version, image...) maybe?
Wdyt??
| hashableDeployment.Spec.Template.Spec.Containers = containers | ||
| hashableDeployment.Spec.Template.Spec.Containers = deployment.Spec.Template.Spec.InitContainers |
There was a problem hiding this comment.
Will this not cause the deployment hash values to be overriden?
| if haproxyVersion != "" { | ||
| env = append(env, corev1.EnvVar{ | ||
| Name: "ROUTER_HAPROXY_VERSION", | ||
| Value: haproxyVersion, | ||
| }) | ||
| } | ||
|
|
There was a problem hiding this comment.
What happens if an invalid version string is given. Is there a fallback or some validation?
| Command: []string{"haproxy"}, | ||
| Args: []string{"-W", "-db", "-S", "/var/lib/haproxy/run/master.sock,mode,600", "-f", "/var/lib/haproxy/conf/haproxy.config"}, | ||
| Lifecycle: &corev1.Lifecycle{ |
There was a problem hiding this comment.
This works on the assumption that haproxy is in the $PATH right? If so what if it is not or has a different config path?
| updated.Spec.Template.Spec.Containers = containers | ||
|
|
||
| // initContainers always override all fields | ||
| updated.Spec.Template.Spec.InitContainers = expected.Spec.Template.Spec.InitContainers |
|
/assign |
add two new annotations used to parameterize the haproxy version to be used in the router.
haproxy-version: defines the haproxy version to use. I must be one of the installed versions in the default router image. If missing, it defaults to the older installed version.haproxy-image: URL of the haproxy image to use. If configured, ingress operator deploys haproxy as a sidecar and configures it instead of the embedded haproxy from the router image.