Sort manifests by Kind and set namespace like helm

open-cluster-management-io / addon-framework

addon apis

Apache License 2.0

23 stars 40 forks source link

Sort manifests by Kind and set namespace like helm #220

Closed tamalsaha closed 5 months ago

tamalsaha commented 10 months ago

In helm charts the namespace might not always be set in the manifest template. But it can be set using --namespace flag. This pr always sets the agentInstallNamespace to the objects rendered from helm charts
Helm will sort objects by kind when applying to a cluster. This pr also follows/copies the same code to sort rendered objects.

qiujian16 commented 10 months ago

cc @zhiweiyin318

zhiweiyin318 commented 10 months ago

@tamalsaha could you add some additional unit tests ?
general LGTM

qiujian16 commented 10 months ago

/approve /lgtm /hold

It seems good to me @zhiweiyin318 would you take a final check?

openshift-ci[bot] commented 10 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: qiujian16, tamalsaha

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/open-cluster-management-io/addon-framework/blob/main/OWNERS)~~ [qiujian16] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment

tamalsaha commented 10 months ago

Please Hold! Don't Merge.

tamalsaha commented 10 months ago

Some resolution is needed for this issue.

https://github.com/open-cluster-management-io/ocm/issues/323

zhiweiyin318 commented 10 months ago

Could you add unit tests or e2e tests for the new codes to check if the ns is added to the objects?

openshift-ci[bot] commented 7 months ago

New changes are detected. LGTM label has been removed.

tamalsaha commented 6 months ago

Hello, I am going to split this into 3 prs so that the agreed upon parts can be merged first.

first pr: https://github.com/open-cluster-management-io/addon-framework/pull/248 second pr: https://github.com/open-cluster-management-io/addon-framework/pull/249

qiujian16 commented 6 months ago

/hold

sorry, I thought it is ready

openshift-merge-robot commented 6 months ago

PR needs rebase.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.