Skip to content
This repository has been archived by the owner on Oct 13, 2021. It is now read-only.

option to ignore cluster-scoped resources #141

Closed
wants to merge 1 commit into from

Conversation

terinjokes
Copy link
Contributor

When running an namespace-scoped instance of Faros in a shared cluster,
listing and modifying cluster-scoped resources may not be allowed. As
Faros begins by listing cluster-scoped resources, this prevents Faros
from operating entirely.

This changelist adds a new flag to the controller --namespaced-only
which modifies the behavior of Faros:

  1. Faros no longer lists ClusterGitTrackObjects at controller startup.
    Any previously created ClusterGitTrackObjects are abandoned and must
    be cleaned up by the operator.
  2. Faros no longer manages any cluster-scoped resources it finds in
    a Git repository, instead adding them to the ignored objects list.

Fixes: #138

When running an namespace-scoped instance of Faros in a shared cluster,
listing and modifying cluster-scoped resources may not be allowed. As
Faros begins by listing cluster-scoped resources, this prevents Faros
from operating entirely.

This changelist adds a new flag to the controller `--namespaced-only`
which modifies the behavior of Faros:

1. Faros no longer lists ClusterGitTrackObjects at controller startup.
   Any previously created ClusterGitTrackObjects are abandoned and must
   be cleaned up by the operator.
2. Faros no longer manages any cluster-scoped resources it finds in
   a Git repository, instead adding them to the ignored objects list.

Fixes: pusher#138
@pusher-ci
Copy link

Hi @terinjokes. Thanks for your PR.

I'm waiting for a pusher member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@terinjokes
Copy link
Contributor Author

I'm having some difficulties adding tests to the controller suite due to the global nature of the farosflags package. Consider this a WIP pull request in the meantime.

Copy link
Contributor

@JoelSpeed JoelSpeed left a comment

Choose a reason for hiding this comment

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

Apart from the lack of tests, I think this is good! I will have a look at how we can reasonably add tests and get back to you

@@ -492,6 +497,11 @@ func (r *ReconcileGitTrack) ignoreObject(u *unstructured.Unstructured) (bool, st
return false, "", err
}

// Ignore namespaced objects in namespaced-only mode
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this comment is not quite right?

Suggested change
// Ignore namespaced objects in namespaced-only mode
// Ignore non-namespaced objects in namespaced-only mode

@JoelSpeed
Copy link
Contributor

I've added a couple of tests as a start to a branch, might be worth cherry-picking those commits and adding some more tests, WDYT?

@JoelSpeed
Copy link
Contributor

/ok-to-test

@pusher-ci
Copy link

@terinjokes: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-faros-test-1.12 fb92eb7 link /test 1.12

Full PR test history

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@pusher-ci
Copy link

@terinjokes: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@terinjokes terinjokes closed this Nov 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Development

Successfully merging this pull request may close these issues.

cannot disable clustergittrackobjects CRD
4 participants