Skip to content

Bug 1715370: Some orphaned logs sent to .operations.* index by mistake. #230

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-robot merged 1 commit into from
Aug 20, 2019
Merged

Bug 1715370: Some orphaned logs sent to .operations.* index by mistake. #230

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions manifests/4.2/cluster-logging.v4.2.0.clusterserviceversion.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ spec:
- configmaps
- secrets
- serviceaccounts
- serviceaccounts/finalizers
Copy link
Contributor

@richm richm Aug 19, 2019

Choose a reason for hiding this comment

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

@jcantrill @ewolinetz do we need to add this rbac anywhere else? Is this CSV the one used by customers?

verbs:
- "*"
- apiGroups:
Expand Down
60 changes: 49 additions & 11 deletions pkg/k8shandler/collection.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,13 @@ import (
"github.com/sirupsen/logrus"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/util/retry"

logging "github.com/openshift/cluster-logging-operator/pkg/apis/logging/v1"
apps "k8s.io/api/apps/v1"
core "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

const (
Expand All @@ -29,19 +32,23 @@ var (
timeout = time.Second * 1800
)

var serviceAccountLogCollectorUID types.UID

//CreateOrUpdateCollection component of the cluster
func (clusterRequest *ClusterLoggingRequest) CreateOrUpdateCollection() (err error) {

cluster := clusterRequest.cluster

var collectorServiceAccount *core.ServiceAccount

// there is no easier way to check this in golang without writing a helper function
// TODO: write a helper function to validate Type is a valid option for common setup or tear down
if cluster.Spec.Collection.Logs.Type == logging.LogCollectionTypeFluentd || cluster.Spec.Collection.Logs.Type == logging.LogCollectionTypeRsyslog {
if err = clusterRequest.createOrUpdateCollectionPriorityClass(); err != nil {
return
}

if err = clusterRequest.createOrUpdateCollectorServiceAccount(); err != nil {
if collectorServiceAccount, err = clusterRequest.createOrUpdateCollectorServiceAccount(); err != nil {
return
}
}
Expand Down Expand Up @@ -162,6 +169,15 @@ func (clusterRequest *ClusterLoggingRequest) CreateOrUpdateCollection() (err err
return
}
}
if collectorServiceAccount != nil && (cluster.Spec.Collection.Logs.Type == logging.LogCollectionTypeFluentd || cluster.Spec.Collection.Logs.Type == logging.LogCollectionTypeRsyslog) {

// remove our finalizer from the list and update it.
collectorServiceAccount.ObjectMeta.Finalizers = utils.RemoveString(collectorServiceAccount.ObjectMeta.Finalizers, metav1.FinalizerDeleteDependents)
err = clusterRequest.Create(collectorServiceAccount)
if err != nil && !errors.IsAlreadyExists(err) {
return
}
}

return nil
}
Expand All @@ -180,17 +196,31 @@ func (clusterRequest *ClusterLoggingRequest) createOrUpdateCollectionPriorityCla
return nil
}

func (clusterRequest *ClusterLoggingRequest) createOrUpdateCollectorServiceAccount() error {
func (clusterRequest *ClusterLoggingRequest) createOrUpdateCollectorServiceAccount() (*core.ServiceAccount, error) {

cluster := clusterRequest.cluster

collectorServiceAccount := NewServiceAccount("logcollector", cluster.Namespace)

utils.AddOwnerRefToObject(collectorServiceAccount, utils.AsOwner(clusterRequest.cluster))

err := clusterRequest.Create(collectorServiceAccount)
if err != nil && !errors.IsAlreadyExists(err) {
return fmt.Errorf("Failure creating Log Collector service account: %v", err)
delfinalizer := false
if collectorServiceAccount.ObjectMeta.DeletionTimestamp.IsZero() {
// This object is not being deleted.
if !utils.ContainsString(collectorServiceAccount.ObjectMeta.Finalizers, metav1.FinalizerDeleteDependents) {
collectorServiceAccount.ObjectMeta.Finalizers = append(collectorServiceAccount.ObjectMeta.Finalizers, metav1.FinalizerDeleteDependents)
}
err := clusterRequest.Create(collectorServiceAccount)
if err != nil && !errors.IsAlreadyExists(err) {
return nil, fmt.Errorf("Failure creating Log Collector service account: %v", err)
}
if len(collectorServiceAccount.ObjectMeta.UID) != 0 {
serviceAccountLogCollectorUID = collectorServiceAccount.ObjectMeta.UID
Copy link
Contributor Author

@nhosoi nhosoi Aug 16, 2019

Choose a reason for hiding this comment

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

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

}
} else if utils.ContainsString(collectorServiceAccount.ObjectMeta.Finalizers, metav1.FinalizerDeleteDependents) {
// This object is being deleted.
// our finalizer is present, so lets handle any dependency
delfinalizer = true
}

// Also create the role and role binding so that the service account has host read access
Expand All @@ -209,9 +239,9 @@ func (clusterRequest *ClusterLoggingRequest) createOrUpdateCollectorServiceAccou

utils.AddOwnerRefToObject(collectorRole, utils.AsOwner(cluster))

err = clusterRequest.Create(collectorRole)
err := clusterRequest.Create(collectorRole)
if err != nil && !errors.IsAlreadyExists(err) {
return fmt.Errorf("Failure creating Log collector privileged role: %v", err)
return nil, fmt.Errorf("Failure creating Log collector privileged role: %v", err)
}

subject := NewSubject(
Expand All @@ -233,7 +263,7 @@ func (clusterRequest *ClusterLoggingRequest) createOrUpdateCollectorServiceAccou

err = clusterRequest.Create(collectorRoleBinding)
if err != nil && !errors.IsAlreadyExists(err) {
return fmt.Errorf("Failure creating Log collector privileged role binding: %v", err)
return nil, fmt.Errorf("Failure creating Log collector privileged role binding: %v", err)
}

// create clusterrole for logcollector to retrieve metadata
Expand All @@ -247,7 +277,7 @@ func (clusterRequest *ClusterLoggingRequest) createOrUpdateCollectorServiceAccou
)
clusterRole, err := clusterRequest.CreateClusterRole("metadata-reader", clusterrules, cluster)
if err != nil {
return err
return nil, err
}
subject = NewSubject(
"ServiceAccount",
Expand All @@ -268,10 +298,14 @@ func (clusterRequest *ClusterLoggingRequest) createOrUpdateCollectorServiceAccou

err = clusterRequest.Create(collectorReaderClusterRoleBinding)
if err != nil && !errors.IsAlreadyExists(err) {
return fmt.Errorf("Failure creating Log collector %q cluster role binding: %v", collectorReaderClusterRoleBinding.Name, err)
return nil, fmt.Errorf("Failure creating Log collector %q cluster role binding: %v", collectorReaderClusterRoleBinding.Name, err)
}

return nil
if delfinalizer {
return collectorServiceAccount, nil
} else {
return nil, nil
}
}

func isDaemonsetDifferent(current *apps.DaemonSet, desired *apps.DaemonSet) (*apps.DaemonSet, bool) {
Expand Down Expand Up @@ -387,3 +421,7 @@ func isBufferFlushRequired(current *apps.DaemonSet, desired *apps.DaemonSet) boo

return (currVersion == "3.11" && desVersion == "4.0.0")
}

func getServiceAccountLogCollectorUID() types.UID {
return serviceAccountLogCollectorUID
}
10 changes: 9 additions & 1 deletion pkg/k8shandler/fluentd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,15 @@ func (clusterRequest *ClusterLoggingRequest) createOrUpdateFluentdDaemonset() (e
fluentdPodSpec := newFluentdPodSpec(cluster, "elasticsearch", "elasticsearch")

fluentdDaemonset := NewDaemonSet("fluentd", cluster.Namespace, "fluentd", "fluentd", fluentdPodSpec)
utils.AddOwnerRefToObject(fluentdDaemonset, utils.AsOwner(cluster))

uid := getServiceAccountLogCollectorUID()
if len(uid) == 0 {
// There's no uid for logcollector serviceaccount; setting ClusterLogging for the ownerReference.
utils.AddOwnerRefToObject(fluentdDaemonset, utils.AsOwner(cluster))
} else {
// There's a uid for logcollector serviceaccount; setting the ServiceAccount for the ownerReference with blockOwnerDeletion.
utils.AddOwnerRefToObject(fluentdDaemonset, NewLogCollectorServiceAccountRef(uid))
}

err = clusterRequest.Create(fluentdDaemonset)
if err != nil && !errors.IsAlreadyExists(err) {
Expand Down
11 changes: 9 additions & 2 deletions pkg/k8shandler/rsyslog.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func (clusterRequest *ClusterLoggingRequest) removeRsyslog() (err error) {
}

// Wait longer than the default terminationGracePeriodSeconds
time.Sleep(12* time.Second)
time.Sleep(12 * time.Second)

if err = clusterRequest.RemoveSecret("rsyslog"); err != nil {
return
Expand Down Expand Up @@ -296,7 +296,14 @@ func (clusterRequest *ClusterLoggingRequest) createOrUpdateRsyslogDaemonset() (e

rsyslogDaemonset := NewDaemonSet("rsyslog", cluster.Namespace, "rsyslog", "rsyslog", rsyslogPodSpec)

utils.AddOwnerRefToObject(rsyslogDaemonset, utils.AsOwner(cluster))
uid := getServiceAccountLogCollectorUID()
if len(uid) == 0 {
// There's no uid for logcollector serviceaccount; setting ClusterLogging for the ownerReference.
utils.AddOwnerRefToObject(rsyslogDaemonset, utils.AsOwner(cluster))
} else {
// There's a uid for logcollector serviceaccount; setting the ServiceAccount for the ownerReference with blockOwnerDeletion.
utils.AddOwnerRefToObject(rsyslogDaemonset, NewLogCollectorServiceAccountRef(uid))
}

err = clusterRequest.Create(rsyslogDaemonset)
if err != nil && !errors.IsAlreadyExists(err) {
Expand Down
17 changes: 17 additions & 0 deletions pkg/k8shandler/serviceaccount.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/openshift/cluster-logging-operator/pkg/utils"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/types"

core "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -44,10 +45,26 @@ func (clusterRequest *ClusterLoggingRequest) RemoveServiceAccount(serviceAccount

serviceAccount := NewServiceAccount(serviceAccountName, clusterRequest.cluster.Namespace)

if serviceAccountName == "logcollector" {
// remove our finalizer from the list and update it.
serviceAccount.ObjectMeta.Finalizers = utils.RemoveString(serviceAccount.ObjectMeta.Finalizers, metav1.FinalizerDeleteDependents)
}

err := clusterRequest.Delete(serviceAccount)
if err != nil && !errors.IsNotFound(err) {
return fmt.Errorf("Failure deleting %v service account: %v", serviceAccountName, err)
}

return nil
}

func NewLogCollectorServiceAccountRef(uid types.UID) metav1.OwnerReference {
return metav1.OwnerReference {
APIVersion: "v1", // apiversion for serviceaccounts/finalizers in cluster-logging.<VER>.clusterserviceversion.yaml
Kind: "ServiceAccount",
Name: "logcollector",
UID: uid,
BlockOwnerDeletion: utils.GetBool(true),
Controller: utils.GetBool(true),
}
}
19 changes: 19 additions & 0 deletions pkg/utils/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,3 +221,22 @@ func CheckFileExists(filePath string) error {
}
return nil
}

func ContainsString(slice []string, s string) bool {
for _, item := range slice {
if item == s {
return true
}
}
return false
}

func RemoveString(slice []string, s string) (result []string) {
for _, item := range slice {
if item == s {
continue
}
result = append(result, item)
}
return
}