Skip to content

Conversation

@stlaz
Copy link
Contributor

@stlaz stlaz commented Sep 13, 2019

Introduces the watchdog for operator deployment so that there is
a mechanism which guards changes to the trusted-ca-bundle CM and
these are refetched by having the operator pod restart.

cc @enj @sttts

@openshift-ci-robot
Copy link
Contributor

@stlaz: This pull request references Bugzilla bug 1751147, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

Bug 1751147: operator: restart the operator pod on trusted-ca-bundle change

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 openshift-ci-robot added bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 13, 2019
@stlaz
Copy link
Contributor Author

stlaz commented Sep 13, 2019

/hold
untested

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 13, 2019
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: stlaz

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 13, 2019
Introduces the watchdog for operator deployment so that there is
a mechanism which guards changes to the trusted-ca-bundle CM and
these are refetched by having the operator pod restart.
@stlaz stlaz force-pushed the refetch_system_trust branch from 41673eb to 3915c83 Compare September 13, 2019 14:22
@openshift-ci-robot
Copy link
Contributor

@stlaz: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
ci/prow/e2e-aws-operator 3915c83 link /test e2e-aws-operator
ci/prow/e2e-aws 3915c83 link /test e2e-aws
ci/prow/e2e-aws-console-login 3915c83 link /test e2e-aws-console-login
ci/prow/e2e-aws-upgrade 3915c83 link /test e2e-aws-upgrade

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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.

Copy link
Contributor

@enj enj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly looks fine but needs a review from someone more familiar with the watch dog code.


rt, err := transportFor("", caData, nil, nil)
// FIXME: this reads too often, either always merge system-trust store in transportForInner or keep this in memory
systemCaData, _ := ioutil.ReadFile("/etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Read the file once in an init block and store in var. You will get restarted anyway when it changes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also this does assume a newline at the end of the first byte slice.

terminationMessagePolicy: FallbackToLogsOnError
resources:
requests:
memory: 50Mi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems high

@stlaz
Copy link
Contributor Author

stlaz commented Sep 17, 2019

It seems that the watchdog is actually wired inside the code already which I did not know.

Superseeded by #195

@stlaz stlaz closed this Sep 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants