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

jg/json endpoint #11

Merged
merged 5 commits into from
Jan 26, 2019
Merged

jg/json endpoint #11

merged 5 commits into from
Jan 26, 2019

Conversation

JessicaGreben
Copy link
Contributor

@JessicaGreben JessicaGreben commented Jan 5, 2019

This is the first pass at adding a json endpoint to return results of validating existing pods in the cluster. There will need to be refactoring (and tests), but I want to make sure this is in alignment with what we want before I make more changes. This works and is currently deployed to the cluster.

This PR does the following:

  • Adds a http webserver
  • Adds a route that runs existing validation on all pods that exist in the cluster and then returns json output of the results
  • Adds another container to the fairwinds deployment that runs this webserver

Copy link
Contributor

@robscott robscott left a comment

Choose a reason for hiding this comment

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

This is really cool! Glad to see this all working. It would be nice if there were some simple tests that could be added along with this against this functionality just so we can get in the habit of adding tests with each new feature.


var clientset = createClientset()

// Kubernetes version 1.11 APIs
Copy link
Contributor

Choose a reason for hiding this comment

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

Why specify the specific API version here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah i don't really need to, let me remove that

return clientset
}

func getKubeConfig() string {
Copy link
Contributor

Choose a reason for hiding this comment

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

controller-runtime's config.getConfigOrDie() function should probably suffice here, and is already in use. Ideally that would also mean there was no need to add an additional package. https://github.com/reactiveops/fairwinds/blob/master/main.go#L55

image: quay.io/reactiveops/fairwinds
command: ["fairwinds", "--webhook"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it valid just to run a single container with both --webhook and --dashboard flags? Both ports could be specified in the container spec.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah i can do that, will just need to make a start up script or use a process manager, but that seems easy enough.


func validateHandler(w http.ResponseWriter, r *http.Request, c conf.Configuration) {
var results []validator.Results
pods, err := kube.CoreV1API.Pods("").List(metav1.ListOptions{})
Copy link
Contributor

Choose a reason for hiding this comment

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

At some point in the future it would be really nice to group/validate these results by deployment/daemon set/stateful set/etc to simplify the validation and output.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah def, i think the next step will be to validate deployments vs pods.

Copy link
Contributor

@robscott robscott left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks!

@JessicaGreben JessicaGreben merged commit 8665fd5 into master Jan 26, 2019
@JessicaGreben JessicaGreben deleted the jg/json-endpoint branch January 26, 2019 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants