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

Add an option in kustomize lib to suppress the deprecation warnings. #5452

Open
1 of 2 tasks
liuwh08 opened this issue Nov 16, 2023 · 8 comments · May be fixed by #5466
Open
1 of 2 tasks

Add an option in kustomize lib to suppress the deprecation warnings. #5452

liuwh08 opened this issue Nov 16, 2023 · 8 comments · May be fixed by #5466
Assignees
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@liuwh08
Copy link

liuwh08 commented Nov 16, 2023

Eschewed features

  • This issue is not requesting templating, unstuctured edits, build-time side-effects from args or env vars, or any other eschewed feature.

What would you like to have added?

A knob or loger option to suppress the warinning emitted by kustomize lib (not the binary).

Why is this needed?

Currently our binary directly calls kustomize.Build() and each call will write deprecation warning to stderr and our binary calls multiple times of Build(). It looks spamming and probably a bit ominous to our end user.

Can you accomplish the motivating task without this feature, and if so, how?

No, it's hard coded to stderr.

What other solutions have you considered?

The alternative is the deprecation of var but it would take sometime to finish on our side as it's not a full feature parity to repalcements. (We finished the jsonPatch and base to resources pretty quickly.)

Anything else we should know?

No response

Feature ownership

  • I am interested in contributing this feature myself! 🎉
@liuwh08 liuwh08 added the kind/feature Categorizes issue or PR as related to a new feature. label Nov 16, 2023
@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Nov 16, 2023
@natasha41575
Copy link
Contributor

I think it's fine to add the knob for suppressing warnings to kustomize as a library. I don't think we should add this knob to the kustomize binary, but folks who are integrating kustomize as a library into their own tool may have reasons to suppress these, particularly if the use of kustomize is an implementation detail that they might not want to expose to users.

/triage accepted
/kind feature
/help

@k8s-ci-robot
Copy link
Contributor

@natasha41575:
This request has been marked as needing help from a contributor.

Guidelines

Please ensure that the issue body includes answers to the following questions:

  • Why are we solving this issue?
  • To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
  • Does this issue have zero to low barrier of entry?
  • How can the assignee reach out to you for help?

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

I think it's fine to add the knob for suppressing warnings to kustomize as a library. I don't think we should add this knob to the kustomize binary, but folks who are integrating kustomize as a library into their own tool may have reasons to suppress these, particularly if the use of kustomize is an implementation detail that they might not want to expose to users.

/triage accepted
/kind feature
/help

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.

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Nov 16, 2023
@natasha41575 natasha41575 added the good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. label Nov 16, 2023
@davidbaena
Copy link

I implemented a code snippet to try to reproduce the warning traces, however this code does not prints any warning:

import (
	"log"
	"os"

	"sigs.k8s.io/kustomize/api/krusty"
	"sigs.k8s.io/kustomize/kyaml/filesys"
)

func main() {
	writer := os.Stdout

	k := krusty.MakeKustomizer(krusty.MakeDefaultOptions())

	fSys := filesys.MakeFsOnDisk()
	m, err := k.Run(fSys, "/ABSOLUTE/PATH/kustomization-foo")
	if err != nil {
		log.Fatalf("Error: %v", err)
		return
	}

	yaml, err := m.AsYaml()
	if err != nil {
		log.Fatalf("Error: %v", err)
		return
	}

	_, err = writer.Write(yaml)
	if err != nil {
		log.Fatalf("Error: %v", err)
		return
	}
}

kustomization-foo contains the "hello world" example: https://github.com/kubernetes-sigs/kustomize/tree/master/examples/helloWorld

@natasha41575 Is this the way to import kustomize as a lib? I'm interested on fix this issue

Thanks a lot!

@davidbaena
Copy link

/assign

@davidbaena
Copy link

I reviewed again the code, and perhaps these flags should be suppressed with a knob when used as lib:

https://github.com/davidbaena/kustomize/blob/790ca0e7b66aae32c4ab6258709f15be471b15b4/kustomize/commands/build/build.go#L112

	msg := "Error marking flag '%s' as deprecated: %v"
	err := cmd.Flags().MarkDeprecated(flagReorderOutputName,
		"use the new 'sortOptions' field in kustomization.yaml instead.")
	if err != nil {
		log.Fatalf(msg, flagReorderOutputName, err)
	}
	err = cmd.Flags().MarkDeprecated(managedByFlag,
		"The flag `enable-managedby-label` has been deprecated. Use the `managedByLabel` option in the `buildMetadata` field instead.")
	if err != nil {
		log.Fatalf(msg, managedByFlag, err)
	}

Do you mean these warnings?

@liuwh08
Copy link
Author

liuwh08 commented Nov 27, 2023

It's emiited in https://github.com/kubernetes-sigs/kustomize/blob/master/api/internal/target/kusttarget.go#L70 and directly to stderr. Ideally it should take a logger (like slog.Logger) and the caller could pass a caller here.

@davidbaena
Copy link

Ok, I'm going to implement that solution
Thanks!

@davidbaena
Copy link

This issue has been implemented in this PR:
#5466

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
Development

Successfully merging a pull request may close this issue.

4 participants