Skip to content

Conversation

@alvaroaleman
Copy link
Contributor

What this PR does / why we need it:

Add aa MutatingAdmissionWebook + Defaulting/Validation for MachineDeployment

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

xref: #227

@alvaroaleman alvaroaleman force-pushed the admission-webhook branch 3 times, most recently from 25a9c76 to 3d86147 Compare October 19, 2018 12:28
port: 8085
periodSeconds: 5
---
apiVersion: apps/v1beta2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Until I put this into a dedicated Deployment, the e2e tests always had some failures because of a connection reset when the APIServer tried to call the Webhook

@alvaroaleman
Copy link
Contributor Author

@mrIncompetent This passes tests now, PTAL :)

@mrIncompetent
Copy link
Contributor

Whats the advantage of using 2 separate docker images?
I would prefer just having a single one for simplicity

@alvaroaleman
Copy link
Contributor Author

Whats the advantage of using 2 separate docker images?
I would prefer just having a single one for simplicity

So one image that contains two binaries? Or the hyperkube way of one binary that switches functionality based off of arg #1?

@mrIncompetent
Copy link
Contributor

Whats the advantage of using 2 separate docker images?
I would prefer just having a single one for simplicity

So one image that contains two binaries? Or the hyperkube way of one binary that switches functionality based off of arg #1?

One image containing 2 binaries

@alvaroaleman
Copy link
Contributor Author

One image containing 2 binaries

Done in b981212

@alvaroaleman
Copy link
Contributor Author

/approve

@alvaroaleman alvaroaleman reopened this Oct 19, 2018
@kubermatic-bot kubermatic-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Oct 19, 2018
@alvaroaleman
Copy link
Contributor Author

/lgtm

@kubermatic-bot
Copy link
Contributor

@alvaroaleman: you cannot LGTM your own PR.

In response to this:

/lgtm

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.

@mrIncompetent
Copy link
Contributor

/lgtm

@kubermatic-bot kubermatic-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 19, 2018
@kubermatic-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alvaroaleman, mrIncompetent

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 [alvaroaleman,mrIncompetent]

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

@kubermatic-bot kubermatic-bot merged commit 1f2f004 into master Oct 19, 2018
@alvaroaleman alvaroaleman deleted the admission-webhook branch October 19, 2018 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants