Skip to content

Conversation

@staebler
Copy link
Contributor

@staebler staebler commented Dec 5, 2020

The determination about whether a custom CA bundle is being used is assessed when the operator starts. This can lead to incorrect behavior if the kube-cloud-config ConfigMap has not yet been created in the openshit-config-managed namespace when the operator starts. Instead of only looking for the ConfigMap at start-up, these changes will have the operator looking for the ConfigMap on every reconcile and adjusting the yaml for the controller asset accordingly.

@staebler staebler changed the title support custom CA bundle for AWS API WIP: support custom CA bundle for AWS API Dec 5, 2020
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 5, 2020
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: staebler
To complete the pull request process, please assign jsafrane after the PR has been reviewed.
You can assign the PR to them by writing /assign @jsafrane in a comment when ready.

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

Details Needs approval from an approver in each of these files:

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

@staebler
Copy link
Contributor Author

staebler commented Dec 5, 2020

This is my attempt at using replacement strings for the deployment asset that will look for the kube-cloud-config ConfigMap as the operator runs rather than just at initialization.

@staebler staebler force-pushed the aws-c2s-extra-replaces branch 2 times, most recently from 718358c to d5ac38b Compare December 7, 2020 15:04
@staebler staebler changed the title WIP: support custom CA bundle for AWS API Bug 1905119: WIP: support custom CA bundle for AWS API Dec 7, 2020
@openshift-ci-robot openshift-ci-robot added bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. and removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Dec 7, 2020
@openshift-ci-robot
Copy link
Contributor

@staebler: This pull request references Bugzilla bug 1905119, 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
  • bug is open, matching expected state (open)
  • bug target release (4.7.0) matches configured target release for branch (4.7.0)
  • bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)
Details

In response to this:

Bug 1905119: WIP: support custom CA bundle for AWS API

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-robot openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Dec 7, 2020
@openshift-ci-robot
Copy link
Contributor

@staebler: This pull request references Bugzilla bug 1905119, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.7.0) matches configured target release for branch (4.7.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)
Details

In response to this:

Bug 1905119: WIP: support custom CA bundle for AWS API

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.

@staebler staebler changed the title Bug 1905119: WIP: support custom CA bundle for AWS API Bug 1905119: support custom CA bundle for AWS API Dec 7, 2020
@staebler
Copy link
Contributor Author

staebler commented Dec 7, 2020

This PR needs openshift/library-go#965 to merge first as it is currently using a fork of library-go with those changes.

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 7, 2020
@openshift-merge-robot
Copy link
Contributor

@staebler: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-aws-csi d5ac38b link /test e2e-aws-csi

Full PR test history. Your PR dashboard.

Details

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.

Copy link
Member

@bertinatto bertinatto left a comment

Choose a reason for hiding this comment

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

Thanks for following up with this @staebler! I think this patch addresses the concerns we had related to the operator startup and templating.

Let's wait for the library-go patch to get in so we can continue here.

Edit: There seems to be some panic going on:

pods/openshift-cluster-csi-drivers_aws-ebs-csi-driver-operator-6b86c8c97-dzltt_aws-ebs-csi-driver-operator.log.gz:E1207 15:34:43.495828       1 runtime.go:78] Observed a panic: &errors.errorString{s:"v1.Deployment.Spec: v1.DeploymentSpec.Template: v1.PodTemplateSpec.Spec: v1.PodSpec.Volumes: []v1.Volume: v1.Volume.VolumeSource: ConfigMap: v1.ConfigMapVolumeSource.Optional: ReadBool: expect t or f, but found \", error found in #10 byte of ...|ptional\":\"${true}\"},|..., bigger context ...|onfigMap\":{\"name\":\"kube-cloud-config\",\"optional\":\"${true}\"},\"name\":\"ca-bundle\"},{\"emptyDir\":{},\"name|..."} (v1.Deployment.Spec: v1.DeploymentSpec.Template: v1.PodTemplateSpec.Spec: v1.PodSpec.Volumes: []v1.Volume: v1.Volume.VolumeSource: ConfigMap: v1.ConfigMapVolumeSource.Optional: ReadBool: expect t or f, but found ", error found in #10 byte of ...|ptional":"${true}"},|..., bigger context ...|onfigMap":{"name":"kube-cloud-config","optional":"${true}"},"name":"ca-bundle"},{"emptyDir":{},"name|...)```

Bump to the latest version of library-go to get changes
to allow for custom string replaces on the CSI controller
asset.
The determination about whether a custom CA bundle is being used
is assessed when the operator starts. This can lead to incorrect
behavior if the kube-cloud-config ConfigMap has not yet been created
in the openshit-config-managed namespace when the operator starts.
Instead of only looking for the ConfigMap at start-up, these changes
will have the operator looking for the ConfigMap on every reconcile
and adjusting the yaml for the controller asset accordingly.
@staebler staebler force-pushed the aws-c2s-extra-replaces branch from d5ac38b to f9208d0 Compare December 21, 2020 21:41
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 21, 2020

@staebler: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-aws-csi f9208d0 link /test e2e-aws-csi

Full PR test history. Your PR dashboard.

Details

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.

@bertinatto
Copy link
Member

@staebler do you think you could use the hooks that was added recently to library-go?

Example: 5f697e6

@staebler
Copy link
Contributor Author

@staebler do you think you could use the hooks that was added recently to library-go?

Example: 5f697e6

/close in favor of #111

@staebler
Copy link
Contributor Author

/close

@openshift-ci-robot
Copy link
Contributor

@staebler: Closed this PR.

Details

In response to this:

/close

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-robot
Copy link
Contributor

@staebler: This pull request references Bugzilla bug 1905119. The bug has been updated to no longer refer to the pull request using the external bug tracker.

Details

In response to this:

Bug 1905119: support custom CA bundle for AWS API

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants