Bug 2017565: [OCPCLOUD-1306] Add user defined trusted CA from cloud-config to ccm-trusted-ca bundle #140
Conversation
…ests Test for cloud-conf sync controller was extended for ensure that all keys are handled and synced
openshift-ci
bot
added
the
bugzilla/severity-high
|
@lobziik: This pull request references Bugzilla bug 2015493, which is invalid:
Comment In response to this:
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. |
openshift-ci
bot
added
the
bugzilla/invalid-bug
|
/test unit |
|
/test e2e-aws-ccm-install |
lobziik
force-pushed
the
cloud-config-ca
branch
from
d2a3554
to
b435d95
Compare
openshift-ci
bot
added
the
approved
|
seems like the unit tests broke |
|
Apparently we need to count on missed cloud config as on expected situation. Also unit-tests are not stable, there is some cleanup issues, didn't figure out why exactly this happen. |
openshift-ci
bot
added
the
do-not-merge/hold
lobziik
changed the title
openshift-ci
bot
added
bugzilla/severity-medium
|
@lobziik: This pull request references Bugzilla bug 2017565, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. 3 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
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. |
openshift-ci
bot
added
bugzilla/valid-bug
lobziik
force-pushed
the
cloud-config-ca
branch
from
e3e33eb
to
196934d
Compare
|
no worries, thanks for the extra detail @lobziik |
openshift-ci
bot
removed
the
approved
|
I fixed tests in 494fc0b |
openshift-ci
bot
removed
the
do-not-merge/hold
lobziik
force-pushed
the
cloud-config-ca
branch
from
0d96a17
to
494fc0b
Compare
|
/retest |
openshift-ci
bot
added
the
approved
| allCMs := &corev1.ConfigMapList{} | ||
| Expect(cl.List(ctx, allCMs)).To(Succeed()) | ||
| for _, cm := range allCMs.Items { | ||
| Expect(cl.Delete(ctx, &cm, deleteOptions)).To(Succeed()) | ||
| Eventually( | ||
| apierrors.IsNotFound(cl.Get(ctx, client.ObjectKeyFromObject(managedCloudConfig), &corev1.ConfigMap{})), | ||
| ).Should(BeTrue()) | ||
| } | ||
|
|
||
| if syncedCloudConfigMap != nil { | ||
| Expect(cl.Delete(ctx, syncedCloudConfigMap, deleteOptions)).To(Succeed()) | ||
| Eventually( | ||
| apierrors.IsNotFound(cl.Get(ctx, client.ObjectKeyFromObject(syncedCloudConfigMap), &corev1.Namespace{})), | ||
| apierrors.IsNotFound(cl.Get(ctx, client.ObjectKeyFromObject(&cm), &corev1.ConfigMap{})), |
There was a problem hiding this comment.
As mentioned in slack, but for the benefit or future selves. You could also use a DeleteAllOf call here, which will delete all of the configmaps in a particular namespace. For example this could be done:
By("Cleanup resources")
for _, ns := range []string{targetNamespaceName, OpenshiftConfigNamespace, OpenshiftManagedConfigNamespace} {
Expect(cl.DeleteAllOf(ctx, &corev1.ConfigMap{},
client.GracePeriodSeconds(0),
client.InNamespace(ns),
)).To(Succeed())
Eventually(func() (bool, error) {
cmList := &corev1.ConfigMapList{}
if err := cl.List(ctx, cmList, client.InNamespace(ns)); err != nil {
return false, err
}
return len(cmList.Items) == 0, nil
},
).Should(BeTrue())
}
7ab49b5
to
4edae3d
Compare
|
I squashed commits and cleaned up tests for cloud-conf syncer a bit, IMHO it's ready for final review. |
|
/retest |
Additional trusted CA from cloud-config should also be counted during ccm-trusted-ca sync procedure. Due to OCP installer nuances, 'additionalTrustBundle' not always ends up in the proxy object. For handling such situation this patch introduces support of 'ca-bundle.pem' key in cloud-config. CA from there will be added to ccm-trusted-ca along with additional CA bundle which reffered by proxy object. However, cloud-config might not be always presented. On some platforms (AWS), there might be no cloud-config at all. So, missed cloud config does not considering as an error. For additional context see: openshift/installer#5251 (comment)
lobziik
force-pushed
the
cloud-config-ca
branch
from
4edae3d
to
372db61
Compare
|
@lobziik: The following tests failed, say
Full PR test history. Your PR dashboard. 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. |
|
/test e2e-aws-ccm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: elmiko, Fedosin 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:
Approvers can indicate their approval by writing |
|
@lobziik: All pull requests linked via external trackers have merged: Bugzilla bug 2017565 has been moved to the MODIFIED state. In response to this:
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. |
Additional trusted CA from cloud-config should also be counted during
ccm-trusted-ca sync procedure. Due to OCP installer nuances,
'additionalTrustBundle' not always ends up in the proxy object.
For handling such situation this patch introduces support of 'ca-bundle.pem'
key in cloud-config. CA from there will be added to ccm-trusted-ca along with
additional CA bundle which reffered by proxy object.
For additional context see:
openshift/installer#5251 (comment)