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

Add cilium helm and kind cluster installation #1444

Merged
merged 12 commits into from
Mar 10, 2025

Conversation

hmahmood
Copy link
Contributor

What does this PR do?

Adds a new variant to a remote kind cluster that supports cilium. There is also a new helm installation component for cilium added.

Which scenarios this will impact?

Motivation

Additional Notes

@hmahmood hmahmood requested a review from a team as a code owner February 25, 2025 22:56
Comment on lines 37 to 43
func init() {
var err error
kindCiliumClusterTemplate, err = template.ParseFS(kindCilumClusterFS, "kind-cilium-cluster.yaml")
if err != nil {
panic(err)
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Is there a real advantage to put it there instead of in NewCiliumKindCluster function. So that it is not executed when not useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to NewCiliumKindCluster

return false
}

func NewHelmInstallation(e config.Env, params *Params, opts ...pulumi.ResourceOption) (*HelmComponent, error) {
Copy link
Member

Choose a reason for hiding this comment

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

I think it can make sense that NewHelmInstallation take a kubernetes cluster as an argument, because it needs information about the cluster to work properly
func NewHelmInstallation(e config.Env, cluster *componentskube.Cluster, params *Params, opts ...pulumi.ResourceOption)
That way it is explicit that you need to give the cluster for it to work. And you can have that: https://github.com/DataDog/datadog-agent/pull/34398/files#diff-9dbae0537bf500d77990a2ef3697b49f572cd967cca5f890c84058eea75885b6R133-R134 in this function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added the parameter

@hmahmood hmahmood force-pushed the hasan.mahmood/add-cilium branch from 22aa96d to c9a011b Compare March 3, 2025 17:31
@hmahmood
Copy link
Contributor Author

/merge

@dd-devflow
Copy link

dd-devflow bot commented Mar 10, 2025

View all feedbacks in Devflow UI.
2025-03-10 22:52:19 UTC ℹ️ Start processing command /merge


2025-03-10 22:52:23 UTC ℹ️ MergeQueue: pull request added to the queue

The median merge time in main is 20m.


2025-03-10 23:13:45 UTC ℹ️ MergeQueue: This merge request was merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants