Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added port to webhook-service, generated bundle with v1.0.1 #1

Conversation

jpkrohling
Copy link

@jpkrohling jpkrohling commented Oct 2, 2020

Based on the discussion from the operator-framework mailing list, I forked your repository and ran make bundle, resulting in the diff from this PR.

Note that I'm using operator-sdk v1.0.1 and controller-gen v0.3.0.

Signed-off-by: Juraci Paixão Kröhling [email protected]

@jpkrohling jpkrohling marked this pull request as draft October 2, 2020 08:39
containerPort: 443
targetPort: 4343
deploymentName: webhook-operator-webhook
- admissionReviewVersions: null
Copy link
Author

Choose a reason for hiding this comment

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

I guess this explains why I'm seeing an error when deploying my operator :-)

Copy link
Owner

Choose a reason for hiding this comment

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

This is fixed in version 1.0.1 of the operator SDK.

Copy link
Author

Choose a reason for hiding this comment

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

This one seems to have missed the v1.0.1 train.

Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Owner

Choose a reason for hiding this comment

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

My mistake - the commit isn't in v1.0.1 of the SDK.

@@ -185,15 +193,11 @@ spec:
- UPDATE
resources:
- webhooktests
sideEffects: None
sideEffects: null
Copy link
Author

Choose a reason for hiding this comment

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

And once I manually fix the admissionReviewVersions in my bundle, I get a message about this field being invalid as well.

Copy link
Owner

Choose a reason for hiding this comment

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

This is fixed in version 1.0.1 of the operator SDK.

Copy link
Author

Choose a reason for hiding this comment

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

This too :-)

@@ -16,3 +16,4 @@ spec:
namespace: webhook-operator-system
name: webhook-service
path: /convert
port: 443
Copy link
Author

Choose a reason for hiding this comment

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

Without this, I get a message like this:

WARN[0000] Warning: Value webhook.operators.coreos.io/v2, Kind=WebhookTest: provided API should have an example annotation 
ERRO[0000] Error: Field spec.conversion.webhookClientConfig.service.port, Value 0: spec.conversion.webhookClientConfig.service.port: Invalid value: 0: port is not valid: must be between 1 and 65535, inclusive 
make: *** [Makefile:115: bundle] Error 1

Copy link
Owner

Choose a reason for hiding this comment

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

This is a bug that I'll look into - we should be defaulting to 443.

@jpkrohling
Copy link
Author

I also just tried using controller-gen 0.2.5, which is what the Makefile in this branch seems to install when it's absent, but it didn't generate a bundle significantly different than this PR:

diff --git a/bundle/manifests/webhook.operators.coreos.io_webhooktests.yaml b/bundle/manifests/webhook.operators.coreos.io_webhooktests.yaml
index 77146f9..568c71b 100644
--- a/bundle/manifests/webhook.operators.coreos.io_webhooktests.yaml
+++ b/bundle/manifests/webhook.operators.coreos.io_webhooktests.yaml
@@ -3,7 +3,7 @@ kind: CustomResourceDefinition
 metadata:
   annotations:
     cert-manager.io/inject-ca-from: webhook-operator-system/webhook-operator-serving-cert
-    controller-gen.kubebuilder.io/version: v0.3.0
+    controller-gen.kubebuilder.io/version: v0.2.5
   creationTimestamp: null
   name: webhooktests.webhook.operators.coreos.io
 spec:
diff --git a/config/crd/bases/webhook.operators.coreos.io_webhooktests.yaml b/config/crd/bases/webhook.operators.coreos.io_webhooktests.yaml
index 1bb7b2e..6d69c2c 100644
--- a/config/crd/bases/webhook.operators.coreos.io_webhooktests.yaml
+++ b/config/crd/bases/webhook.operators.coreos.io_webhooktests.yaml
@@ -4,7 +4,7 @@ apiVersion: apiextensions.k8s.io/v1beta1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.3.0
+    controller-gen.kubebuilder.io/version: v0.2.5
   creationTimestamp: null
   name: webhooktests.webhook.operators.coreos.io
 spec:

@jpkrohling
Copy link
Author

@awgreene sorry for pinging you directly, but did you have a chance to take a look at this one?

@jpkrohling jpkrohling closed this Oct 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants