Drop unnecessary steps and better logging when generating docs

[Kavinjsir]

What do you want to happen?

Background

1. Unnecessary docker commands

Currently executing make generate-docs will trigged docker rmi -f, which might not be necessary:

2. Hard to debugging when error happens during docs generation

Another case is: when error happens during docs generation by make generate-docs, it is hard to locate the error position from the display log info:

Suggestion

1. Remove the docker command and other unnecessary steps if possible for make generate-docs
2. Display rich log info when generating docs.

One Approach for suggestion 2

For the UpdateTutorial method, we can change the comment lines to log.Info:

So that when running make generate-docs, it support trace the process:

Extra Labels

/kind documentation, /kind cleanup

[k8s-ci-robot]

@Kavinjsir: The label(s) kind/documentation,, kind//kind cannot be applied, because the repository doesn't have them.

In response to this:

What do you want to happen?

Background

1. Unnecessary docker commands

Currently executing make generate-docs will trigged docker rmi -f, which might not be necessary:

2. Hard to debugging when error happens during docs generation

Another case is: when error happens during docs generation by make generate-docs, it is hard to locate the error position from the display log info:

Suggestion

1. Remove the docker command and other unnecessary steps if possible for make generate-docs
2. Display rich log info when generating docs.

One Approach for suggestion 2

For the UpdateTutorial method, we can change the comment lines to log.Info:

So that when running make generate-docs, it support trace the process:

Extra Labels

/kind documentation, /kind cleanup

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.

[ashutosh887]

@Kavinjsir
Should I work on this one?

[camilamacedo86]

+1, IMO we need to:

• Improve the logs as suggested
• Ensure that all operations has a log for errors with an unique text so that when it fails we know where and the error itself.

I am adding a good first issue on this one.

@ashutosh887,

If you be able to help on this one would be great. 🥇
Please feel free to do so.

[ashutosh887]

Sure let me work on this @camilamacedo86

Thanks

[Sajiyah-Salat]

as descirbed by @Kavinjsir I have reprroduced the issue.
here's the output:

Generating documents...
Generating component-config tutorial...
INFO[0000] Generating the sample context of component-config... 
INFO[0000] destroying directory for sample project      
  running: docker rmi -f 
  warning: docker rmi -f  failed with error: (chdir docs/book/src/component-config-tutorial/testdata/project/: no such file or directory) 
INFO[0000] refreshing tools and creating directory...   
  cleaning up tools
ERRO[0000] error creating directory for sample project: unlinkat /snap/bin/kustomize: permission denied 
exit status 1

as I am more of a beginner I checked the gnereate_samples.go and couldnt find any print statement that print about docker. Also I have chekced two three related makefile but couldnt find the same. Can you please help me locate the file where changes are needed? Also through this pr we will try to add stack traces of which file and line have produced the error so user have ease as described above by @Kavinjsir

also the component config tutorial is deprecated? right. Then why are we solving the issue. Sorry for the noob questions.

[camilamacedo86]

What is required to be done in this task:

1) What is the main go of this task:

The main go of this task is ensure that for each operation done in the hack implementation to generate the doc samples we have a check to verify the errors with an information that help us to know what fails so that we can fix it. For example, see:

kubebuilder/hack/docs/internal/cronjob-tutorial/generate_cronjob.go

Lines 469 to 475 in c2fbb3e

	CheckError("fixing suite_test.go", err)
	err = pluginutil.InsertCode(
	filepath.Join(sp.ctx.Dir, "internal/controller/suite_test.go"),
	`import (`, `
	"context"`)
	CheckError("fixing suite_test.go", err)

The both operations has an message fixing suite_test.go so that if we see this error we do not know where it fails.
Therefore, the whole main point is to improve the code implementation to help us debug and troubleshooting when/if required.

2) Second expected improvement

See the description docker command and other unnecessary steps.
It occurs because we are using the same methods used to do the tests and it is required in the e2e tests.
See:

kubebuilder/test/e2e/utils/test_context.go

Lines 241 to 251 in c2fbb3e

	// Destroy is for cleaning up the docker images for testing
	func (t *TestContext) Destroy() {
	//nolint:gosec
	cmd := exec.Command("docker", "rmi", "-f", t.ImageName)
	if _, err := t.Run(cmd); err != nil {
	warnError(err)
	}
	if err := os.RemoveAll(t.Dir); err != nil {
	warnError(err)
	}
	}

Therefore, in the sample case we call Destroy but we do not need to call cmd := exec.Command("docker", "rmi", "-f", t.ImageName) So we can created a new method DestroyTempDir and use it instead or just replace the Destroy usage in the hack docs per the cmd commands and use it directly.

NOTE: It is recommended to create one PR for each improvement so that it will make easier the review process.

[Sajiyah-Salat]

Sure. I will first focus on the first task.
we will add filename, line, err.
Do you think we also want to give different message or let the message as it is?

[Sajiyah-Salat]

Also can you please let me know how should I check my changes.

[Sajiyah-Salat]

I have done some research on my own it looks like withstack function can be used to debug errors more efficiently. i will try if succeded I will try to send the pr.

[Kavinjsir]

I think we may also take care about: making sure that the execution of generate-docs should use the kubebuilder built from the same source code.

For now, the code for generation assumes the env provides kubebuilder, so it does not check if this "kubebuilder" is created from the source code.

I guess this might lead to confusion when we have updates the document to sync with certain plugins updates.
For me, I have to remind myself to run make install before make generate.

[Sajiyah-Salat]

So you mean that we should first update the kubebuilder repository and then generate docs which will be easy for users as they dont need to remember to run make install before make generate. right?

[varshaprasad96]

Agree that this is an issue, that we are directly using the kubebuilder available in the path, rather than building from the source code. However, we would have look for the approach we are following while generating testdata - if we are running the make install command directly, or if we have a helper in place to build the KB binary.

/assign @Kavinjsir