Support 'ignore' mode in ConfigReconciler.

[sophieliu15]

This PR adds the support for 'ignore' mode in ConfigReconciler. Specifically, it adds following features:

• If the mode of a type is set to 'ignore', the corresponding object reconciler will ignore all subsequent reconciliations (e.g., new or changed objects will not be propagated).
• If the mode of a type is changed from 'ignore' to 'propagate', reconciliations for all existing objects of the type in the cluster will be triggered.
• If a mode of a type is unrecognized, it will be treated as 'ignore'.
• If a mode of a type is unset, it will be treated as 'propagate'.

Tested: GKE cluster; unit tests.

Design doc: http://bit.ly/hnc-type-configuration
Issue: #411

[k8s-ci-robot]

Hi @sophieliu15. Thanks for your PR.

I'm waiting for a kubernetes-sigs 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.

[sophieliu15]

@adrianludwin and @yiqigao217 PTAL. As usual, I cannot change existing reviewers.

[adrianludwin]

I'll review this when the other change goes in but for now:

/ok-to-test

[adrianludwin]

/lgtm
/approve
/hold
/assign @rjbez17

[rjbez17]

Bear with my dumb question here, but can you confirm the workflow in the following scenario?

1. I deploy HNC, I set the Config to Remove objects of type Foo.Bar
2. Foo.Bar Object types DNE on the cluster so it is set to Ignore
3. I deploy a CRD of type Foo.Bar into the cluster
4. ?

Looking at the code, it seems that Foo.Bar will be locked into Ignore until you essentially 'touch' the HNC Config. Is that correct? Is that desired? (I could have missed something in the code, I haven't had coffee yet)

I'm less concerned about Remove here as it wouldn't really matter in this usecase. But if we add more mode types in the future, I'd like to understand the flow.

Similarly, it'd seem that in step 1, if I explicitly set Foo.Bar to Propagate it would end up being Ignore while if I did not explicitly set it, it would be set to Propagate.

If we self correct without a manual 'touch' here then just ignore me :)

[sophieliu15]

Hi @rjbez17

IIUC, is your concern about configuring a custom resource in the HNCConfiguration object instead of build-in resource (e.g., roles, rolebindings, secrets)? That is a good point. I haven't considered supporting customized resource in
HNCConfiguration yet. Right now I am assuming the CRD exists before adding the type in the config (i.e., mostly I only consider types we previously supported in gvk.go). I experimented the workflow with the CronTab custom resource as defined here. Please see my comments below about current behaviors and desired behaviors.

Bear with my dumb question here, but can you confirm the workflow in the following scenario?

1. I deploy HNC, I set the Config to Remove objects of type Foo.Bar

At this point, if Foo.Bar is a random type that hasn't been defined (i.e., CRD does not exist), we will not create an object reconciler for it.
For the config object, it will have an entry in Status indicating the object reconciler creation failed. Here is an example, CronTab is a random type that I haven't defined yet.

Spec:
  Types:
    API Version:  v1
    Kind:         CronTab
    Mode:         remove
    API Version:  rbac.authorization.k8s.io/v1
    Kind:         Role
    Mode:         propagate
    API Version:  rbac.authorization.k8s.io/v1
    Kind:         RoleBinding
    Mode:         propagate
Status:
  Conditions:
    Code:  objectReconcilerCreationFailed
    Msg:   Couldn't create ObjectReconciler for type stable.example.com/v1, Kind=CronTab: no matches for kind "CronTab" in version "stable.example.com/v1"

2. Foo.Bar Object types DNE on the cluster so it is set to Ignore

We do not change the spec of config object, so if you look at the content of config object (above), you will see the Mode for Foo.Bar is what you set before (i.e., remove). In terms of the "mode" of object reconciler (you can think of that as the behavior of the object reconciler or the Mode field of the ObjectReconciler struct), it is not set to anything because the object reconciler does not even exists.

3. I deploy a CRD of type Foo.Bar into the cluster

The object reconciler for Foo.Bar still does not exist; therefore, if you start to create Foo.Bar objects, it will not be propagated. This is the same behavior as ignore mode if the reconciler exists.

From my experiment, even you retouch the config object to trigger the reconciliation of ConfigReconciler, the object reconciler for Foo.Bar still cannot be created with the same error. Only when I reinstall HNC (make deploy), the object reconciler will be created.

Once the object reconciler is created, the mode of that object reconciler will be ignore because we treat remove as an unrecognized mode in this PR (see getValidateMode in object.go), but it will be remove mode once we support remove.

4. ?

Looking at the code, it seems that Foo.Bar will be locked into Ignore until you essentially 'touch' the HNC Config. Is that correct? Is that desired? (I could have missed something in the code, I haven't had coffee yet)

The Foo.Bar objects will not be reconciled because the object reconciler for Foo.Bar does not exist. We will create object reconciler for it not after we retouch config object, but after we reinstall HNC.

This may not be ideal but I feel the solution might more on the documentation side than technical side. For example, we can add in the user guide that:

• Only add type in config object if the type exists or
• Reinstall HNC if they add a new CRD while the HNC is running.

At the same time, we can also add a feature when we implement the admission controller to automatically remove undefined type from the spec (not sure if this is feasible), but this does not solve the issue that we need to reinstall HNC to recognize custom resource defined while running HNC.

I'm less concerned about Remove here as it wouldn't really matter in this usecase. But if we add more mode types in the future, I'd like to understand the flow.

Similarly, it'd seem that in step 1, if I explicitly set Foo.Bar to Propagate it would end up being Ignore while if I did not explicitly set it, it would be set to Propagate.

For step 1, if the Foo.Bar type does not exist, the mode in Spec will be whatever you set it to be. However, this does not matter since the object reconciler for Foo.Bar does not exist.

If the type exists, Propagate and Ignore will still be Propagate and Ignore. If it is not set, it will be Propagate, anything else will be Ignore (including Remove until we start to support that mode). See details in getValidateMode in object.go

If we self correct without a manual 'touch' here then just ignore me :)

Feel free to let me know if there are any questions or your thoughts about this issue.

(Also add @adrianludwin in the discussion for his thoughts)

[adrianludwin]

From my experiment, even you retouch the config object to trigger the reconciliation of ConfigReconciler, the object reconciler for Foo.Bar still cannot be created with the same error. Only when I reinstall HNC (make deploy), the object reconciler will be created.

This is a bug, and I'm pretty sure we can find a way to fix it - let's just file an issue for now. For example, we can add a watch onto CRD types - or maybe just use the webhook to stop it from happening. You can also always just kill the manager pod to restart HNC, you don't need to fully reinstall it.

[adrianludwin]

@rjbez17 , did Sophie answer your questions?

[sophieliu15]

From my experiment, even you retouch the config object to trigger the reconciliation of ConfigReconciler, the object reconciler for Foo.Bar still cannot be created with the same error. Only when I reinstall HNC (make deploy), the object reconciler will be created.

This is a bug, and I'm pretty sure we can find a way to fix it - let's just file an issue for now. For example, we can add a watch onto CRD types - or maybe just use the webhook to stop it from happening. You can also always just kill the manager pod to restart HNC, you don't need to fully reinstall it.

Fired #488 to track this issue.

[rjbez17]

/lgtm
/approve
/hold cancel

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adrianludwin, rjbez17, sophieliu15

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 [adrianludwin,rjbez17]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment