Skip to content
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

Jump to bottom

Switch the fluentd configmap to one-per-Revision #1474

Closed
mattmoor opened this issue Jul 3, 2018 · 0 comments
Closed

Switch the fluentd configmap to one-per-Revision #1474

mattmoor opened this issue Jul 3, 2018 · 0 comments
Assignees
@mattmoor
Labels
area/API API objects and controllers area/monitoring kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt.

Comments

@mattmoor
Copy link
Member

mattmoor commented Jul 3, 2018

Expected Behavior

Controlled resources are 1:1 with the Revision, e.g. Deployment / Service for user code, Deployment / Service for the Autoscaler.

Actual Behavior

The fluentd ConfigMap tries to be 1-per-namespace, which actually complicates the controller logic a bunch. We can't use shared utilities, we have to do careful reference count bookkeeping, ...

We should just manage this like other resources, and we can optimize it later if the redundancy becomes problematic.
@google-prow-robot google-prow-robot added area/API API objects and controllers area/monitoring kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. labels Jul 3, 2018
@mattmoor mattmoor self-assigned this Jul 5, 2018
@mattmoor mattmoor mentioned this issue Jul 5, 2018
Merged
google-prow-robot pushed a commit that referenced this issue Jul 6, 2018
@mattmoor @google-prow-robot
Make the fluentd ConfigMap a more conventional resource. (#1499)
b230afa 
This makes the handling of the Fluentd ConfigMap in the Revision controller more like the other resources (1:1 with Revision).
 * Naming is handled by `pkg/controller/revision/resources/names`
 * Creating of our "desired" ConfigMap is done through `pkg/controller/revision/resources`
 * The reconciliation in `pkg/controller/revision/revision.go` follows the pattern:

```
  got, err := lister.Things(namespace).Get(name)
  if IsNotFound(err) {
    want := resources.MakeThing(rev)
    got, err = client.Create(want)
    if err != nil {
      return err
    }
  } else if err != nil {
    return err
  } else {
    want := resources.MakeThing(rev)
    if !equality.Semantic.DeepEqual(got, want) {
      got.Spec = want.Spec
      got, err = client.Update(got)
      if err != nil {
        return err
      }
    }
  }

  // Optionally propagate status based on the state of "got"
```

Fixes: #1474
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mattmoor mattmoor

Labels
area/API API objects and controllers area/monitoring kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@mattmoor @google-prow-robot