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.

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

balloons: allow assigning containers to balloons by runtime-evaluated expressions. #260

Merged

Conversation

klihub
Copy link
Collaborator

@klihub klihub commented Feb 9, 2024

Notes:

Implement assigning containers to balloon instances by container match expression defined in balloon definitions. Match expressions are evaluated for containers which are not explicitly assigned to any balloon by an annotation, before namespace-based assignments. If an expression in a balloon definition matches a container, that container is assigned to a balloon instance of that definition.

For instance, given this balloon definition among the configuration

    ...
      balloonTypes:
      - name: specialBalloon
        matchContainers:
          - key: pod/labels/app.kubernetes.io/component
            operator: Equals
            values: [ "SpecialComponent" ]
    ...

any container not explicitly annotated to some other balloon will get assigned to instances of specialBalloon, if the pod of the container has the label app.kubernetes.io/component=SpecialComponent

Similarly, given this balloon definition among the configuration

    ...
      balloonTypes:
      - name: podBalloon
        matchContainers:
          - key: pod/name
            operator: In
            values: [ "pod1", "pod2", "pod3" ]
    ...

all containers of pods named pod1, pod2, and pod3 will be assigned to instances of podBalloon.

@klihub klihub requested review from kad and askervin February 9, 2024 17:12
@klihub klihub force-pushed the devel/assign-balloons-by-expression branch 2 times, most recently from 02f490f to b940530 Compare February 12, 2024 21:53
@klihub klihub marked this pull request as ready for review February 12, 2024 21:56
@klihub klihub force-pushed the devel/assign-balloons-by-expression branch from caec3d1 to 1703d85 Compare February 12, 2024 22:37
Copy link
Collaborator

@askervin askervin left a comment

Choose a reason for hiding this comment

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

Great stuff! Just few comments.

config/crd/bases/config.nri_balloonspolicies.yaml Outdated Show resolved Hide resolved
cmd/plugins/balloons/policy/balloons-policy.go Outdated Show resolved Hide resolved
pkg/apis/resmgr/v1alpha1/expression.go Outdated Show resolved Hide resolved
pkg/apis/resmgr/v1alpha1/expression.go Outdated Show resolved Hide resolved
@klihub klihub force-pushed the devel/assign-balloons-by-expression branch from 1703d85 to 36aff44 Compare February 13, 2024 09:05
@klihub klihub force-pushed the devel/assign-balloons-by-expression branch from 36aff44 to 1f1a98f Compare February 13, 2024 09:17
Copy link
Collaborator

@askervin askervin left a comment

Choose a reason for hiding this comment

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

Spotted one typo in the doc. Otherwise LGTM.

And I'm ashamed of my previous key[0] panic comment: the short-circuiting || took care of the problem. Brightness of the moment when thinking "here could be a potential problem" seems to be blinding.

docs/resource-policy/policy/balloons.md Outdated Show resolved Hide resolved
@klihub klihub force-pushed the devel/assign-balloons-by-expression branch 4 times, most recently from 30cd686 to cccfe59 Compare February 13, 2024 10:27
@klihub klihub force-pushed the devel/assign-balloons-by-expression branch 2 times, most recently from e7c6a2a to 2e1bf8d Compare February 13, 2024 14:54
@klihub
Copy link
Collaborator Author

klihub commented Feb 13, 2024

Spotted one typo in the doc. Otherwise LGTM.

And I'm ashamed of my previous key[0] panic comment: the short-circuiting || took care of the problem. Brightness of the moment when thinking "here could be a potential problem" seems to be blinding.

I got fooled by your comment for a moment, too. So there is not need to be... 🙈

@klihub klihub requested a review from askervin February 13, 2024 15:24
@klihub klihub force-pushed the devel/assign-balloons-by-expression branch from 2e1bf8d to 78085b1 Compare February 13, 2024 15:29
Copy link
Collaborator

@askervin askervin left a comment

Choose a reason for hiding this comment

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

LGTM.

@klihub klihub force-pushed the devel/assign-balloons-by-expression branch 4 times, most recently from 0477c87 to 9e01c9e Compare February 14, 2024 08:15
Implement assigning containers to balloon instances by
container match expression defined in balloon definitions.
Match expressions are evaluated for containers which are
not explicitly assigned to any balloon by an annotation,
before namespace-based assignments. If an expression in a
balloon definition matches a container, that container is
assigned to a balloon instance of that definition.

For instance, given this balloon definition among the
configuration

...
  balloonTypes:
  - name: specialBalloon
    matchExpressions:
      - key: pod/labels/app.kubernetes.io/component
        operator: Equals
        values: [ "SpecialComponent" ]
...

any container not explicitly annotated to some other balloon
will get assigned to instances of specialBalloon, if the pod
of the container has the label
  app.kubernetes.io/component=SpecialComponent

Similarly, given this balloon definition among the
configuration

...
  balloonTypes:
  - name: podBalloon
    matchExpressions:
      - key: pod/name
        operator: In
        values: [ "pod1", "pod2", "pod3" ]
...

all containers of pods named pod1, pod2, and pod3 will be
assigned to instances of podBalloon.

Signed-off-by: Krisztian Litkey <[email protected]>
Implement custom validation for balloons. The custom validator
validates all container match expressions found in any of the
balloon definitions.

Signed-off-by: Krisztian Litkey <[email protected]>
@klihub klihub force-pushed the devel/assign-balloons-by-expression branch from 9e01c9e to 3b9fbf9 Compare February 14, 2024 08:26
Copy link
Collaborator

@fmuyassarov fmuyassarov left a comment

Choose a reason for hiding this comment

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

LGTM

@ppalucki
Copy link

LGTM 👍

@fmuyassarov fmuyassarov merged commit 516da7f into containers:main Feb 14, 2024
3 checks passed
@klihub klihub deleted the devel/assign-balloons-by-expression branch February 14, 2024 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants