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

Introduce custom ConfigMap for system-probe and move built-in configuration to ENV vars #316

Merged
merged 1 commit into from
Jun 3, 2021

Conversation

vboulineau
Copy link
Contributor

What does this PR do?

Currently the Operator uses config file instead of ENV vars, which means users cannot use a custom configuration file.

Motivation

What inspired you to submit this pull request?

Additional Notes

Anything else we should know when reviewing?

Describe your test plan

Deploy the Operator with:

  • System probe enabled
  • Using inline CustomConfig or using a custom ConfigMap name.

@vboulineau vboulineau added the enhancement New feature or request label May 27, 2021
@vboulineau vboulineau added this to the v0.6 milestone May 27, 2021
@vboulineau vboulineau requested review from a team as code owners May 27, 2021 15:55
Copy link
Contributor

@ruthnaebeck ruthnaebeck left a comment

Choose a reason for hiding this comment

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

Docs review

@@ -3464,6 +3464,30 @@ spec:
to connect to the netlink/conntrack subsystem to add NAT
information to connection data. See also: http://conntrack-tools.netfilter.org/'
type: boolean
customConfig:
description: Allow to put custom configuration for system-probe,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
description: Allow to put custom configuration for system-probe,
description: Enable custom configuration for system-probe,

file content.
type: string
configMap:
description: ConfigMap name of a ConfigMap used to mount
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
description: ConfigMap name of a ConfigMap used to mount
description: The name of a ConfigMap used to mount

content.
type: string
name:
description: Name the ConfigMap name.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
description: Name the ConfigMap name.
description: The name the ConfigMap name.

@@ -3334,6 +3334,29 @@ spec:
to connect to the netlink/conntrack subsystem to add NAT information
to connection data. See also: http://conntrack-tools.netfilter.org/'
type: boolean
customConfig:
description: Allow to put custom configuration for system-probe,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
description: Allow to put custom configuration for system-probe,
description: Enable custom configuration for system-probe,

file content.
type: string
configMap:
description: ConfigMap name of a ConfigMap used to mount
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
description: ConfigMap name of a ConfigMap used to mount
description: The name of a ConfigMap used to mount

content.
type: string
name:
description: Name the ConfigMap name.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
description: Name the ConfigMap name.
description: The name the ConfigMap name.

@vboulineau
Copy link
Contributor Author

@ruthnaebeck I've integrated 2 of your suggestions and entirely rephrased the other one as the pre-existing doc phrase was not clear enough. Let me know if it's good to go.

Copy link
Collaborator

@clamoriniere clamoriniere left a comment

Choose a reason for hiding this comment

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

few small nits, otherwise looks good

@vboulineau vboulineau merged commit fdbdd7f into main Jun 3, 2021
@vboulineau vboulineau deleted the vboulineau/sysprobe branch June 3, 2021 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants