Bug 1821095: Try to promote inactive routes following route deletion #126
Conversation
openshift-ci-robot
added
the
bugzilla/severity-medium
|
@Miciah: This pull request references Bugzilla bug 1821095, which is valid. The bug has been updated to refer to the pull request using the external bug tracker. 3 validation(s) were run on this bug
In 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 kubernetes/test-infra repository. |
openshift-ci-robot
added
the
bugzilla/valid-bug
openshift-ci-robot
added
the
approved
Miciah
force-pushed
the
delete-activation
branch
2 times, most recently
from
1861ea8
to
3a75917
Compare
|
@Miciah: This pull request references Bugzilla bug 1821095, which is valid. 3 validation(s) were run on this bug
In 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 kubernetes/test-infra repository. |
openshift-ci-robot
added
the
needs-rebase
Before this patch, inactive routes were not being promoted when the conflicting routes were deleted. Fix it so that when deletes happen, all inactive routes are given a chance to be promoted.
Fix the paths for the certificate and CA certificate and whitelist directories. Commit c2f1bbd changed the router's working directory from /var/lib/haproxy/router to /var/lib/haproxy but did not update these directory paths, which are relative to the working directory path. Follow-up to commit c2f1bbd. * images/router/haproxy/conf/haproxy-config.template: * images/router/nginx/conf/nginx-config.template: Add "router/" to certificate directory paths. * pkg/router/template/router.go (certDir, caCertDir): Prepend "router/". (whitelistDir): New constant with the path for whitelist files. * pkg/router/template/template_helper.go (generateHAProxyCertConfigMap): Use certDir. (generateHAProxyWhiteListFile): Use whitelistDir. * pkg/router/template/template_helper_test.go (TestGenerateHAProxyCertConfigMap, TestGenerateHAProxyMap): Adjust expected paths (relative to the working directory) for certificates.
If writing the default certificate out to disk fails, log the error message and use the certificate in the container image. Before this commit, the router swallowed the error message and used the certificate in the container image on the assumptions that (1) the error could only result from the absence if a configured default certificate and (2) that the absence of a configured default certificate implied that the user did not intend to configure a default certificate. Logging the error makes it easier for the user to understand what is happening if one or both of these assumptions do not hold true. * pkg/router/template/router.go (writeDefaultCert): If writing the default certificate fails, log the error message. Improve the wording of the log message that indicates that the router is using the certificate from the container image.
|
Rebased. |
openshift-ci-robot
removed
the
needs-rebase
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: danehans, Miciah The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@Miciah: Some pull requests linked via external trackers have merged: . The following pull requests linked via external trackers have not merged:
In 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 kubernetes/test-infra repository. |
|
/bugzilla refresh |
|
@Miciah: Some pull requests linked via external trackers have merged: . The following pull requests linked via external trackers have not merged:
In 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 kubernetes/test-infra repository. |
|
/bugzilla refresh |
|
@Miciah: All pull requests linked via external trackers have merged: openshift/router#126. Bugzilla bug 1821095 has been moved to the MODIFIED state. In 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 kubernetes/test-infra repository. |
Delete a reference to routes.json, which openshift/router#126 deleted, when dumping router pod info * test/extended/router/router.go: Delete routes.json from cat command.
Delete a reference to routes.json, which openshift/router#126 deleted, when dumping router pod info. * test/extended/router/router.go: Delete routes.json from cat command.
update deps
Try to promote inactive routes following route deletion
Before this patch, inactive routes were not being promoted when the conflicting routes were deleted. Fix it so that when deletes happen, all inactive routes are given a chance to be promoted.
Fix default template render output paths
Minor cleanup
Fix certificate and whitelist directory paths
Fix the paths for the certificate and CA certificate and whitelist directories. Commit c2f1bbd changed the router's working directory from
/var/lib/haproxy/routerto/var/lib/haproxybut did not update these directory paths, which are relative to the working directory path.Follow-up to commit c2f1bbd.
images/router/haproxy/conf/haproxy-config.template:images/router/nginx/conf/nginx-config.template: Addrouter/to certificate directory paths.pkg/router/template/router.go(certDir,caCertDir): Prependrouter/.(
whitelistDir): New constant with the path for whitelist files.pkg/router/template/template_helper.go(generateHAProxyCertConfigMap): UsecertDir.(
generateHAProxyWhiteListFile): UsewhitelistDir.pkg/router/template/template_helper_test.go(TestGenerateHAProxyCertConfigMap,TestGenerateHAProxyMap): Adjust expected paths (relative to the working directory) for certificates.writeDefaultCert: Log error writing default certIf writing the default certificate out to disk fails, log the error message and use the certificate in the container image.
Before this commit, the router swallowed the error message and used the certificate in the container image on the assumptions that (1) the error could only result from the absence if a configured default certificate and (2) that the absence of a configured default certificate implied that the user did not intend to configure a default certificate. Logging the error makes it easier for the user to understand what is happening if one or both of these assumptions do not hold true.
pkg/router/template/router.go(writeDefaultCert): If writing the default certificate fails, log the error message. Improve the wording of the log message that indicates that the router is using the certificate from the container image.This is #122 with some fix-ups.
@frobware, @danehans