Bug 1715370: Some orphaned logs sent to .operations.* index by mistake. #230
Conversation
nhosoi
added
the
do-not-merge/work-in-progress
openshift-ci-robot
removed
the
do-not-merge/work-in-progress
|
@nhosoi: This pull request references a valid Bugzilla bug. 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. 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 kubernetes/test-infra repository. |
openshift-ci-robot
added
bugzilla/valid-bug
pkg/k8shandler/collection.go
Outdated
There was a problem hiding this comment.
The UID in the logcollector service account is set when it is created. So, I store it to use it in ownerReference later here. If there is an api to retrieve it dynamically, we don't have to store the UID this way... But so far I could not find it.
sample logcollector service account
apiVersion: v1
imagePullSecrets:
- name: logcollector-dockercfg-rgwsp
kind: ServiceAccount
metadata:
creationTimestamp: "2019-08-15T22:12:46Z"
finalizers:
- foregroundDeletion
name: logcollector
namespace: openshift-logging
ownerReferences:
- apiVersion: logging.openshift.io/v1
controller: true
kind: ClusterLogging
name: instance
uid: cec9ff31-bfa9-11e9-b67d-029422b5c914
resourceVersion: "468364"
selfLink: /api/v1/namespaces/openshift-logging/serviceaccounts/logcollector
uid: cfa30bf4-bfa9-11e9-a7b2-0a1781cc6f5c
|
/test e2e-aws |
pkg/k8shandler/collection.go
Outdated
There was a problem hiding this comment.
if delfinalizer {it's already a bool
richm
left a comment
richm
left a comment
There was a problem hiding this comment.
other than a minor nit I don't see any obvious errors
have you tested this with the origin-aggregated-logging CI tests?
In the Cluster Logging instance deletion, resources under Cluster Logging were deleted independently, which could leave a short time period when the fluentd or rsyslog daemonset was still running but its logcollector service account was removed. That made the logs processed in the time miss kubernetes information including the namespace name. In this patch, the following changes are made: - Setting foregroundDeletion finalizer to collector service account - Making the ownerReference of fluentd and rsyslog daemonset the collector service account - Setting blockOwnerDeletion to true in the ownerReference - The foregroundDeletion informs whether the collector service account is being deleted or not via reconciler. If it is being deleted, letting the collector service account deletion wait until the other objects are all removed, then, delete the finalizer and delete the service account.
Yes! (^o^)/ |
| - configmaps | ||
| - secrets | ||
| - serviceaccounts | ||
| - serviceaccounts/finalizers |
There was a problem hiding this comment.
@jcantrill @ewolinetz do we need to add this rbac anywhere else? Is this CSV the one used by customers?
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: nhosoi, richm The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@nhosoi: All pull requests linked via external trackers have merged. Bugzilla bug 1715370 has been moved to the MODIFIED state. 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 kubernetes/test-infra repository. |
Bug 1715370: Some orphaned logs sent to .operations.* index by mistake.
4 participants
In the Cluster Logging instance deletion, resources under Cluster Logging
were deleted independently, which could leave a short time period when the
fluentd or rsyslog daemonset was still running but its logcollector service
account was removed. That made the logs processed in the time miss kubernetes
information including the namespace name.
In this patch, the following changes are made:
or not via reconciler. If it is being deleted, letting the collector service account
deletion wait until the other objects are all removed, then, delete the finalizer and
delete the service account.