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

📖 Update the Kubebuilder book tutorials for v3 #2302

Conversation

hickeyma
Copy link
Contributor

This PR updates the Kubebuilder book tutorials to align with the current version, v3. This fix will help users navigate the book with success.

Fixes #2273

@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Aug 18, 2021
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 18, 2021
@hickeyma hickeyma changed the title 📖 Update the Kubebuilder book tutorials for v3 :book Update the Kubebuilder book tutorials for v3 Aug 18, 2021
@hickeyma hickeyma changed the title :book Update the Kubebuilder book tutorials for v3 📖 Update the Kubebuilder book tutorials for v3 Aug 18, 2021
@hickeyma hickeyma marked this pull request as draft August 18, 2021 17:11
@k8s-ci-robot k8s-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 Aug 18, 2021
@hickeyma hickeyma changed the title 📖 Update the Kubebuilder book tutorials for v3 📖 WIP: Update the Kubebuilder book tutorials for v3 Aug 18, 2021
@hickeyma
Copy link
Contributor Author

This is a WIP as I traverse the book. The initial docs (intro, quick start and arch) are reviewed. Next will be the cronjob tutorial.

@hickeyma
Copy link
Contributor Author

@hickeyma hickeyma force-pushed the fix/sync-crontab-tutorial-withbook branch 4 times, most recently from a7260e1 to 220128c Compare August 25, 2021 15:20
@hickeyma
Copy link
Contributor Author

Unit test failing. Just wondering if this is a glitch or not?

STEP: tearing down the test environment
Failure [20.007 seconds]
[AfterSuite] AfterSuite 
/home/runner/work/kubebuilder/kubebuilder/docs/book/src/cronjob-tutorial/testdata/project/controllers/suite_test.go:152

  Unexpected error:
      <errors.aggregate | len:1, cap:1>: [
          <*errors.errorString | 0xc0008499d0>{
              s: "timeout waiting for process kube-apiserver to stop",
          },
      ]
      timeout waiting for process kube-apiserver to stop
  occurred

  /home/runner/work/kubebuilder/kubebuilder/docs/book/src/cronjob-tutorial/testdata/project/controllers/suite_test.go:155

@hickeyma hickeyma force-pushed the fix/sync-crontab-tutorial-withbook branch 4 times, most recently from 0d8aee2 to 1df7a31 Compare August 27, 2021 16:08
@hickeyma
Copy link
Contributor Author

Had to disable some unit tests because they are failing.

I disabled https://github.com/kubernetes-sigs/kubebuilder/blob/master/docs/book/src/cronjob-tutorial/testdata/project/controllers/suite_test.go#L130-L145 because it is failing as follows:

STEP: tearing down the test environment
Failure [20.007 seconds]
[AfterSuite] AfterSuite 
/home/runner/work/kubebuilder/kubebuilder/docs/book/src/cronjob-tutorial/testdata/project/controllers/suite_test.go:152

  Unexpected error:
      <errors.aggregate | len:1, cap:1>: [
          <*errors.errorString | 0xc0008499d0>{
              s: "timeout waiting for process kube-apiserver to stop",
          },
      ]
      timeout waiting for process kube-apiserver to stop
  occurred

  /home/runner/work/kubebuilder/kubebuilder/docs/book/src/cronjob-tutorial/testdata/project/controllers/suite_test.go:155

I disabled https://github.com/kubernetes-sigs/kubebuilder/blob/master/docs/book/src/cronjob-tutorial/testdata/project/controllers/cronjob_controller_test.go#L186-L198 as it is failing as follows:

• Failure [22.098 seconds]
CronJob controller
/Users/mhickey/tmp/project/controllers/cronjob_controller_test.go:53
  When updating CronJob Status
  /Users/mhickey/tmp/project/controllers/cronjob_controller_test.go:66
    Should increase CronJob Status.Active count when new Jobs are created [It]
    /Users/mhickey/tmp/project/controllers/cronjob_controller_test.go:67

    Timed out after 10.001s.
    should list our active job test-job in the active jobs list in status
    Expected
        <[]string | len:0, cap:0>: []
    to consist of
        <[]string | len:1, cap:1>: ["test-job"]
    the missing elements were
        <[]string | len:1, cap:1>: ["test-job"]

    /Users/mhickey/tmp/project/controllers/cronjob_controller_test.go:198
------------------------------
STEP: By creating a new CronJob
STEP: By checking the CronJob has zero active Jobs
STEP: By creating a new Job
STEP: By checking that the CronJob has one active Job



Summarizing 1 Failure:

[Fail] CronJob controller When updating CronJob Status [It] Should increase CronJob Status.Active count when new Jobs are created 
/Users/mhickey/tmp/project/controllers/cronjob_controller_test.go:198

Ran 1 of 1 Specs in 43.234 seconds
FAIL! -- 0 Passed | 1 Failed | 0 Pending | 0 Skipped

@hickeyma
Copy link
Contributor Author

@camilamacedo86 I would appreciate your feedback when you get a chance. Note the tests disabled as described in #2302 (comment).

@hickeyma hickeyma marked this pull request as ready for review August 31, 2021 16:39
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 31, 2021
Signed-off-by: Martin Hickey <martin.hickey@ie.ibm.com>
@hickeyma hickeyma force-pushed the fix/sync-crontab-tutorial-withbook branch from 1df7a31 to fbc6a30 Compare September 1, 2021 14:56
@hickeyma
Copy link
Contributor Author

hickeyma commented Sep 1, 2021

/retest

@camilamacedo86
Copy link
Member

HI @hickeyma,

We need to solve : kubernetes-sigs/controller-runtime#1571

@hickeyma
Copy link
Contributor Author

hickeyma commented Sep 3, 2021

Thanks for the feedback @camilamacedo86. Blocked waiting on #2325

Signed-off-by: Martin Hickey <martin.hickey@ie.ibm.com>
@hickeyma hickeyma force-pushed the fix/sync-crontab-tutorial-withbook branch from 4c852eb to 0c4ffef Compare September 7, 2021 13:35
@hickeyma hickeyma changed the title 📖 WIP: Update the Kubebuilder book tutorials for v3 📖 Update the Kubebuilder book tutorials for v3 Sep 7, 2021
@hickeyma
Copy link
Contributor Author

hickeyma commented Sep 7, 2021

@camilamacedo86 I updated the failing tests and they are now passing. I have updated the controller-runtime envTest issue in kubernetes-sigs/controller-runtime#1571 (comment). I had to change how the test environment is brought down when a controller is started. I have updated cronjob_tutorial test in this PR accordingly.

Signed-off-by: Martin Hickey <martin.hickey@ie.ibm.com>
Copy link
Contributor

@estroz estroz left a comment

Choose a reason for hiding this comment

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

The tutorial envtest bits will need to be changed when teardown starts working properly again. For now the comments are explanatory enough.

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 8, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: estroz, hickeyma

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 8, 2021
@k8s-ci-robot k8s-ci-robot merged commit 9817db7 into kubernetes-sigs:master Sep 8, 2021
@hickeyma hickeyma deleted the fix/sync-crontab-tutorial-withbook branch September 9, 2021 08:17
@hickeyma
Copy link
Contributor Author

hickeyma commented Sep 9, 2021

Thanks for the reviews @camilamacedo86 @estroz, and for merging @estroz.

@estroz
Copy link
Contributor

estroz commented Sep 9, 2021

Thanks for your contribution!

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. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Code generated by kubebuilder init doesn't match code in book
4 participants