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

feature: default filters vs opt-in #158

Closed
1 task
AlexsJones opened this issue Mar 30, 2023 · 2 comments · Fixed by #161
Closed
1 task

feature: default filters vs opt-in #158

AlexsJones opened this issue Mar 30, 2023 · 2 comments · Fixed by #161
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@AlexsJones
Copy link
Member

AlexsJones commented Mar 30, 2023

Checklist:

  • [x ] I've searched for similar issues and couldn't find anything matching
  • [x ] I've discussed this feature in the #k8sgpt slack channel

Is this feature request related to a problem?

  • Yes
  • [ x] No

As we start to build out more capabilities, there are those that are not necessarily catching "errors" but more best-practice or good guidance on how to operate with Kubernetes. One such example os PDB ( Pod disruption budget ), it is not an error not to have this on a deployment/sts/ds but it is good practice for when you wish to roll a small deployment without loss of availability.

I propose that we have a mechanism to list/set default filters

e.g.

k8sgpt filters list
> Pod
> Service
> Ingress
etc..

Then you could add or remove a filter

k8sgpt filters add: <Resource> 
// Check if its found
k8sgpt filters remove: <Resource>
// Check if its found

This would then form the basis of a new array we would pull from viper and loop over
e.g.

func RunAnalysis(ctx context.Context, filters []string, config *AnalysisConfiguration,
	client *kubernetes.Client,
	aiClient ai.IAI, analysisResults *[]Analysis) error {

      // HERE WE WOULD BUILD THE ANALYZER MAP BASED OFF OF viper.Get("default_filters")


	// if there are no filters selected then run all of them
	if len(filters) == 0 {  
		for _, analyzer := range analyzerMap { 
			if err := analyzer(ctx, config, client, aiClient, analysisResults); err != nil {
				return err
			}
		}
		return nil
	}

	for _, filter := range filters {
		if analyzer, ok := analyzerMap[filter]; ok {
			if err := analyzer(ctx, config, client, aiClient, analysisResults); err != nil {
				return err
			}
		}
	}
	return nil
}
@AlexsJones AlexsJones added enhancement New feature or request good first issue Good for newcomers labels Mar 30, 2023
@matthisholleville
Copy link
Contributor

As we discussed together, I'm fine with it. I'm interested in proposing an implementation for this issue.

@thschue
Copy link
Contributor

thschue commented Mar 30, 2023

@matthisholleville: Assigned this issue to you

fyuan1316 pushed a commit to fyuan1316/k8sgpt that referenced this issue Jun 26, 2023
…-ai#158)

Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants