openshift / compliance-operator

Operator providing OpenShift cluster compliance checks
Apache License 2.0
110 stars 110 forks source link

Add KubeletConfig Remediation Support #722

Closed Vincent056 closed 3 years ago

Vincent056 commented 3 years ago

Add to support and handle KubeletConfig Remediation object type.

openshift-ci[bot] commented 3 years ago

Hi @Vincent056. Thanks for your PR.

I'm waiting for a openshift 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](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.
JAORMX commented 3 years ago

/ok-to-test

Vincent056 commented 3 years ago

Great start!

This is missing tests and hooking KubeletConfigs to the pausing mechanism in the suite controller.

The testing is working, tho I feel the code is a little repetitive from the machine config remediation

JAORMX commented 3 years ago
     helpers.go:276: compliancesuites.compliance.openshift.io "kubeletconfig-test-node" not found 
Vincent056 commented 3 years ago
     helpers.go:276: compliancesuites.compliance.openshift.io "kubeletconfig-test-node" not found 

Ahh I made a mistake on variable name in tailored profile

JAORMX commented 3 years ago
 E1012 01:30:52.105950    9550 request.go:1001] Unexpected error when reading response body: read tcp 10.131.29.134:41538->54.193.193.30:6443: read: connection reset by peer
    helpers.go:171: E2E-FAILURE: 2021-10-12T01:30:52Z: Error waiting for nodes to reach the worker pool again: unexpected error when reading response body. Please retry. Original error: read tcp 10.131.29.134:41538->54.193.193.30:6443: read: connection reset by peer 

Sounds like a flake and that we should be better prepared for these issues in our e2e tests.

JAORMX commented 3 years ago

/retest

Vincent056 commented 3 years ago

/retest

I think it is due to the machine config pool updates, should we increase the timeout, the e2e did not wait long enough for node to come back up, it happened to e2e test on my local cluster as well. But after e2e, I manually checked all the kubeletconfig got rendered and updated in machine config pool.

JAORMX commented 3 years ago

/retest

error provisioning the cluster

Vincent056 commented 3 years ago
I1012 11:20:46.829255       1 kubelet_config_controller.go:304] Error syncing kubeletconfig compliance-operator-kubelet: could not find any MachineConfigPool set for KubeletConfig
E1012 11:23:14.278960       1 render_controller.go:217] error finding pools for machineconfig: could not find any MachineConfigPool set for MachineConfig 75-test-remediate-e2e-no-direct-root-logins with labels: map[compliance.openshift.io/scan-name:test-remediate-e2e compliance.openshift.io/suite:test-remediate machineconfiguration.openshift.io/role:e2e]
I1012 11:23:30.679120       1 kubelet_config_controller.go:304] Error syncing kubeletconfig compliance-operator-kubelet: could not find any MachineConfigPool set for KubeletConfig

I found from MCO logs, it seems like we don't have label for e2e pool?

Vincent056 commented 3 years ago
 machineConfigPoolSelector:
    matchLabels:
      pools.operator.machineconfiguration.openshift.io/e2e: 

This is the machineConfigPoolSelector I used for KubeletConfig @JAORMX

I think we should add label pools.operator.machineconfiguration.openshift.io/e2e: "" to our e2e pool

JAORMX commented 3 years ago

/retest

JAORMX commented 3 years ago

/retest

JAORMX commented 3 years ago
 machineConfigPoolSelector:
    matchLabels:
      pools.operator.machineconfiguration.openshift.io/e2e: 

This is the machineConfigPoolSelector I used for KubeletConfig @JAORMX

I think we should add label pools.operator.machineconfiguration.openshift.io/e2e: "" to our e2e pool

Good find!

JAORMX commented 3 years ago

/approve

openshift-ci[bot] commented 3 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JAORMX, Vincent056

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/openshift/compliance-operator/blob/master/OWNERS)~~ [JAORMX] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
JAORMX commented 3 years ago

/lgtm