[1.1.x] Fix Citadel using in-memory cert without writing to secret issue. #14510
Conversation
myidpt
requested review from
wenchenglu,
wattli,
JimmyCYJ,
lei-tang and
quanjielin
|
🤔 🐛 You appear to be fixing a bug in Go code, yet your PR doesn't include updates to any test files. Did you forget to add a test? Courtesy of your friendly test nag. |
1 similar comment
|
🤔 🐛 You appear to be fixing a bug in Go code, yet your PR doesn't include updates to any test files. Did you forget to add a test? Courtesy of your friendly test nag. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: myidpt 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 |
|
/test e2e-mixer-no_auth |
myidpt
changed the title
|
/test e2e-mixer-no_auth |
|
@myidpt: The following tests failed, say
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. I understand the commands that are listed here. |
|
prow/e2e-mixer-no_auth.sh is a known failure unrelated to this change |
| secret := BuildSecret("", CASecret, namespace, nil, nil, nil, pemCert, pemKey, istioCASecretType) | ||
| if _, err = client.Secrets(namespace).Create(secret); err != nil { | ||
| log.Errorf("Failed to write secret to CA (error: %s). This CA will not persist when restart.", err) | ||
| log.Errorf("Failed to write secret to CA (error: %s). Abort.", err) | ||
| return nil, fmt.Errorf("failed to create CA due to secret write error") |
There was a problem hiding this comment.
What will happen here? The Citadel process will exit?
There was a problem hiding this comment.
That's correct. The process will exit if the CA-root secret already exists and Citadel tries to overwrite it. In this case, the Citadel will be restarted by Kubelet, to read the CA-root secret again.
I think this is good and safe enough for this fix. We may implement retries in the future if necessary.
|
I agree this is the safest change for now while fixing this issue. Oliver, could you file a github issue for feature improvement to add retry on network timeout? and assign an owner and make sure we keep track of that |
Fix on 1.1 for #14512.
Integration test done by: #14513