-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
WIP: docs: update book mocks #1492
WIP: docs: update book mocks #1492
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: camilamacedo86 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 |
/test pull-kubebuilder-e2e-k8s-1-16-2 |
@mengqiy could you please help by dong a review on this one? |
e8d6e8f
to
b4a1c34
Compare
/test pull-kubebuilder-e2e-k8s-1-15-3 |
WORKDIR / | ||
COPY --from=builder /workspace/manager . | ||
USER nonroot:nonroot | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes were made too long ago and were not reflected here.
@@ -91,7 +96,7 @@ func main() { | |||
Our existing call to SetupWebhookWithManager registers our conversion webhooks with the manager, too. | |||
*/ | |||
if err = (&batchv1.CronJob{}).SetupWebhookWithManager(mgr); err != nil { | |||
setupLog.Error(err, "unable to create webhook", "webhook", "Captain") | |||
setupLog.Error(err, "unable to create webhook", "webhook", "CronJob") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the kind is CronJob
and not Captain
. it is a small fix.
LeaderElection: enableLeaderElection, | ||
LeaderElectionID: "a33bd623.tutorial.kubebuilder.io", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these lines scaffolded by default was missing here.
kind: CronJob | ||
version: v1 | ||
version: 3-alpha |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Upgrade with V3 format
@@ -29,7 +29,7 @@ import ( | |||
) | |||
|
|||
// +kubebuilder:docs-gen:collapse=Go imports | |||
|
|||
// log is for logging in this package. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing scaffolded line by default.
_ = batchv2.AddToScheme(scheme) | ||
utilruntime.Must(kbatchv1.AddToScheme(scheme)) // we've added this ourselves | ||
utilruntime.Must(batchv1.AddToScheme(scheme)) | ||
utilruntime.Must(batchv2.AddToScheme(scheme)) | ||
// +kubebuilder:scaffold:scheme |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done here: #1462
@@ -62,15 +64,18 @@ func main() { | |||
var enableLeaderElection bool | |||
flag.StringVar(&metricsAddr, "metrics-addr", ":8080", "The address the metric endpoint binds to.") | |||
flag.BoolVar(&enableLeaderElection, "enable-leader-election", false, | |||
"Enable leader election for controller manager. Enabling this will ensure there is only one active controller manager.") | |||
"Enable leader election for controller manager. "+ | |||
"Enabling this will ensure there is only one active controller manager.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated with the current scaffolded format.
group: apiextensions.k8s.io | ||
path: spec/conversion/webhookClientConfig/service/namespace | ||
create: false | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing default namespace scaffold for it.
@@ -2,34 +2,45 @@ | |||
# Image URL to use all building/pushing image targets |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
update makefile with the current version.
b4a1c34
to
cf27456
Compare
/test pull-kubebuilder-e2e-k8s-1-17-0 |
cf27456
to
1478909
Compare
/hold We might need to split to make easier the review. |
Close in prol of #1632 |
@camilamacedo86: PR needs rebase. 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. |
Description
Motivation
Closes: The CustomResourceDefinition "cronjobs.batch.tutorial.kubebuilder.io" is invalid #1544