Update fork

README.md

@@ -16,7 +16,7 @@ Once your VPAs are in place, you'll see recommendations appear in the Goldilocks
 <img src="img/screenshot.png" alt="Goldilocks Screenshot" />
 </div>
 
-**Want to learn more?** Reach out on [the Slack channel](https://fairwindscommunity.slack.com/messages/goldilocks) ([request invite](https://join.slack.com/t/fairwindscommunity/shared_invite/zt-cxss92z7-YjfnJwpUwlviViBFjYV2gg)), send an email to `opensource@fairwinds.com`, or join us for [office hours on Zoom](https://fairwindscommunity.slack.com/messages/office-hours)
+**Want to learn more?** Reach out on [the Slack channel](https://fairwindscommunity.slack.com/messages/goldilocks) ([request invite](https://join.slack.com/t/fairwindscommunity/shared_invite/zt-e3c6vj4l-3lIH6dvKqzWII5fSSFDi1g)), send an email to `opensource@fairwinds.com`, or join us for [office hours on Zoom](https://fairwindscommunity.slack.com/messages/office-hours)
 
 ## Requirements
 

cmd/dashboard.go

@@ -41,7 +41,7 @@ var dashboardCmd = &cobra.Command{
 	Long:  `Run the goldilocks dashboard that will show recommendations.`,
 	Run: func(cmd *cobra.Command, args []string) {
 
-		router := dashboard.GetRouter(serverPort, basePath, utils.VpaLabels, excludeContainers)
+		router := dashboard.GetRouter(serverPort, basePath, utils.VPALabels, excludeContainers)
 		http.Handle("/", router)
 		klog.Infof("Starting goldilocks dashboard server on port %d", serverPort)
 		klog.Fatalf("%v", http.ListenAndServe(fmt.Sprintf(":%d", serverPort), nil))

cmd/summary.go

@@ -18,30 +18,51 @@ import (
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
+	"strings"
 
 	"github.com/spf13/cobra"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/klog"
 
 	"github.com/fairwindsops/goldilocks/pkg/summary"
-	"github.com/fairwindsops/goldilocks/pkg/utils"
 )
 
 var excludeContainers string
 var outputFile string
+var namespace string
 
 func init() {
 	rootCmd.AddCommand(summaryCmd)
 	summaryCmd.PersistentFlags().StringVarP(&excludeContainers, "exclude-containers", "e", "", "Comma delimited list of containers to exclude from recommendations.")
 	summaryCmd.PersistentFlags().StringVarP(&outputFile, "output-file", "f", "", "File to write output from audit.")
+	summaryCmd.PersistentFlags().StringVarP(&namespace, "namespace", "n", "", "Limit the summary to only a single Namespace.")
 }
 
 var summaryCmd = &cobra.Command{
 	Use:   "summary",
-	Short: "Genarate a summary of the vpa recommendations in a namespace.",
-	Long:  `Gather all the vpa data in a namespace and generaate a summary of the recommendations.`,
+	Short: "Generate a summary of vpa recommendations.",
+	Long: `Gather all the vpa data generate a summary of the recommendations.
+By default the summary will be about all VPAs in all namespaces.`,
+	Args: cobra.ArbitraryArgs,
 	Run: func(cmd *cobra.Command, args []string) {
+		var opts []summary.Option
+
+		// limit to a single namespace
+		if namespace != "" {
+			opts = append(opts, summary.ForNamespace(args[0]))
+		}
+
+		// exclude containers from the summary
+		if excludeContainers != "" {
+			opts = append(opts, summary.ExcludeContainers(sets.NewString(strings.Split(excludeContainers, ",")...)))
+		}
+
+		summarizer := summary.NewSummarizer(opts...)
+		data, err := summarizer.GetSummary()
+		if err != nil {
+			klog.Fatalf("Error getting summary: %v", err)
+		}
 
-		data, _ := summary.GetInstance().Run(utils.VpaLabels, excludeContainers)
 		summaryJSON, err := json.Marshal(data)
 		if err != nil {
 			klog.Fatalf("Error marshalling JSON: %v", err)

pkg/dashboard/dashboard.go

@@ -19,9 +19,11 @@ import (
 	"encoding/json"
 	"html/template"
 	"net/http"
+	"strings"
 
 	packr "github.com/gobuffalo/packr/v2"
 	"github.com/gorilla/mux"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/klog"
 
 	"github.com/fairwindsops/goldilocks/pkg/summary"
@@ -162,7 +164,12 @@ func GetRouter(port int, basePath string, vpaLabels map[string]string, excludeCo
 			return
 		}
 
-		data, err := summary.GetInstance().Run(vpaLabels, excludeContainers)
+		summarizer := summary.NewSummarizer(
+			summary.ForVPAsWithLabels(vpaLabels),
+			summary.ExcludeContainers(sets.NewString(strings.Split(excludeContainers, ",")...)),
+		)
+
+		data, err := summarizer.GetSummary()
 		if err != nil {
 			klog.Errorf("Error getting data: %v", err)
 			http.Error(w, "Error running summary.", 500)

pkg/summary/options.go

@@ -0,0 +1,50 @@
+package summary
+
+import (
+	"github.com/fairwindsops/goldilocks/pkg/kube"
+	"github.com/fairwindsops/goldilocks/pkg/utils"
+	"k8s.io/apimachinery/pkg/util/sets"
+)
+
+type Option func(*options)
+
+// options for getting and caching the Summarizer's VPAs
+type options struct {
+	kubeClient         *kube.ClientInstance
+	vpaClient          *kube.VPAClientInstance
+	namespace          string
+	vpaLabels          map[string]string
+	excludedContainers sets.String
+}
+
+// defaultOptions for a Summarizer
+func defaultOptions() *options {
+	return &options{
+		kubeClient:         kube.GetInstance(),
+		vpaClient:          kube.GetVPAInstance(),
+		namespace:          namespaceAllNamespaces,
+		vpaLabels:          utils.VPALabels,
+		excludedContainers: sets.NewString(),
+	}
+}
+
+// ForNamespace is an Option for limiting the summary to a single namespace
+func ForNamespace(namespace string) Option {
+	return func(opts *options) {
+		opts.namespace = namespace
+	}
+}
+
+// ExcludeContainers is an Option for excluding containers in the summary
+func ExcludeContainers(excludedContainers sets.String) Option {
+	return func(opts *options) {
+		opts.excludedContainers = excludedContainers
+	}
+}
+
+// ForVPAsWithLabels is an Option for limiting the summary to certain VPAs matching the labels
+func ForVPAsWithLabels(vpaLabels map[string]string) Option {
+	return func(opts *options) {
+		opts.vpaLabels = vpaLabels
+	}
+}

pkg/summary/summary.go

@@ -20,13 +20,22 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apimachinery/pkg/util/sets"
 	v1beta2 "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1beta2"
 	"k8s.io/klog"
 
-	"github.com/fairwindsops/goldilocks/pkg/kube"
 	"github.com/fairwindsops/goldilocks/pkg/utils"
 )
 
+var (
+	labelBase                             = "goldilocks.fairwinds.com"
+	deploymentExcludeContainersAnnotation = labelBase + "/" + "exclude-containers"
+)
+
+const (
+	namespaceAllNamespaces = ""
+)
+
 type containerSummary struct {
 	LowerBound     corev1.ResourceList `json:"lowerBound"`
 	UpperBound     corev1.ResourceList `json:"upperBound"`
@@ -49,119 +58,145 @@ type Summary struct {
 	Namespaces  []string            `json:"namespaces"`
 }
 
-// Client checks if VPA objects should be created or deleted
-// what? how does it do that? it checks it??
-type Client struct {
-	//changing these two to match the naming in here...but should it be consistent?
-	KubeClient    *kube.ClientInstance
-	KubeClientVPA *kube.VPAClientInstance
-}
+// Summarizer represents a source of generating a summary of VPAs
+type Summarizer struct {
+	options
 
-var singleton *Client
+	// cached list of vpas
+	vpas []v1beta2.VerticalPodAutoscaler
+}
 
-// GetInstance returns a Client singleton
-func GetInstance() *Client {
-	if singleton == nil {
-		singleton = &Client{
-			KubeClient:    kube.GetInstance(),
-			KubeClientVPA: kube.GetVPAInstance(),
-		}
+// NewSummarizer returns a Summarizer for all goldilocks managed VPAs in all Namespaces
+func NewSummarizer(setters ...Option) *Summarizer {
+	opts := defaultOptions()
+	for _, setter := range setters {
+		setter(opts)
 	}
-	return singleton
-}
 
-// SetInstance sets the singleton using preconstructed k8s and vpa clients. Used for testing.
-func SetInstance(k8s *kube.ClientInstance, vpa *kube.VPAClientInstance) *Client {
-	singleton = &Client{
-		KubeClient:    k8s,
-		KubeClientVPA: vpa,
+	return &Summarizer{
+		options: *opts,
 	}
-	return singleton
 }
 
-// Run creates a summary of the vpa info for all namespaces.
-func (client *Client) Run(vpaLabels map[string]string, excludeContainers string) (Summary, error) {
-	klog.V(3).Infof("Looking for VPAs with labels: %v", vpaLabels)
-
-	vpaListOptions := metav1.ListOptions{
-		LabelSelector: labels.Set(vpaLabels).String(),
-	}
+// NewSummarizerForVPAs returns a Summarizer for a known list of VPAs
+func NewSummarizerForVPAs(vpas []v1beta2.VerticalPodAutoscaler, setters ...Option) *Summarizer {
+	summarizer := NewSummarizer(setters...)
 
-	vpas, err := client.KubeClientVPA.Client.AutoscalingV1beta2().VerticalPodAutoscalers("").List(vpaListOptions)
-	if err != nil {
-		klog.Error(err.Error())
-	}
-	klog.V(10).Infof("Found vpas: %v", vpas)
+	// set the cached vpas list directly
+	summarizer.vpas = vpas
 
-	summary, _ := GetInstance().constructSummary(vpas, excludeContainers)
-	return summary, nil
+	return summarizer
 }
 
-func (client *Client) constructSummary(vpas *v1beta2.VerticalPodAutoscalerList, excludeContainers string) (Summary, error) {
+// GetSummary returns a Summary of the Summarizer using its options
+func (s Summarizer) GetSummary() (Summary, error) {
 	var summary Summary
-	if len(vpas.Items) <= 0 {
-		return summary, nil
+	// cached vpas
+	if s.vpas == nil {
+		err := s.UpdateVPAs()
+		if err != nil {
+			return summary, err
+		}
 	}
 
-	containerExclusions := strings.Split(excludeContainers, ",")
-
-	for _, vpa := range vpas.Items {
-		klog.V(8).Infof("Analyzing vpa: %v", vpa.ObjectMeta.Name)
+	if len(s.vpas) <= 0 {
+		return summary, nil
+	}
 
-		var deploy deploymentSummary
-		deploy.DeploymentName = vpa.ObjectMeta.Name
-		deploy.Namespace = vpa.ObjectMeta.Namespace
+	summaryNamespaces := sets.NewString()
+	for _, vpa := range s.vpas {
+		klog.V(8).Infof("Analyzing vpa: %v", vpa.Name)
 
-		summary.Namespaces = append(summary.Namespaces, deploy.Namespace)
+		var dSummary deploymentSummary
+		dSummary.DeploymentName = vpa.Name
+		dSummary.Namespace = vpa.Namespace
+		summaryNamespaces.Insert(dSummary.Namespace)
 
-		deployment, err := client.KubeClient.Client.AppsV1().Deployments(deploy.Namespace).Get(deploy.DeploymentName, metav1.GetOptions{})
+		deployment, err := s.kubeClient.Client.AppsV1().Deployments(dSummary.Namespace).Get(dSummary.DeploymentName, metav1.GetOptions{})
 		if err != nil {
 			klog.Errorf("Error retrieving deployment from API: %v", err)
 		}
 
 		if vpa.Status.Recommendation == nil {
-			klog.V(2).Infof("Empty status on %v", deploy.DeploymentName)
+			klog.V(2).Infof("Empty status on %v", dSummary.DeploymentName)
 			continue
 		}
 		if len(vpa.Status.Recommendation.ContainerRecommendations) <= 0 {
-			klog.V(2).Infof("No recommendations found in the %v vpa.", deploy.DeploymentName)
+			klog.V(2).Infof("No recommendations found in the %v vpa.", dSummary.DeploymentName)
 			continue
 		}
 
-		if labelValue, labelFound := deployment.Labels["goldilocks.fairwinds.com/exclude-containers"]; labelFound {
-			containerExclusions = append(containerExclusions, strings.Split(labelValue, ",")...)
+		// get the full set of excluded containers for this Deployment
+		excludedContainers := sets.NewString().Union(s.excludedContainers)
+		if val, exists := deployment.GetAnnotations()[deploymentExcludeContainersAnnotation]; exists {
+			excludedContainers.Insert(strings.Split(val, ",")...)
 		}
 
 	CONTAINER_REC_LOOP:
 		for _, containerRecommendation := range vpa.Status.Recommendation.ContainerRecommendations {
-			for _, exclusion := range containerExclusions {
-				if exclusion == containerRecommendation.ContainerName {
-					klog.V(2).Infof("Excluding container %v", containerRecommendation.ContainerName)
-					continue CONTAINER_REC_LOOP
-				}
+			if excludedContainers.Has(containerRecommendation.ContainerName) {
+				klog.V(2).Infof("Excluding container Deployment/%s/%s", dSummary.DeploymentName, containerRecommendation.ContainerName)
+				continue CONTAINER_REC_LOOP
 			}
 
-			var container = containerSummary{
-				ContainerName:  containerRecommendation.ContainerName,
-				UpperBound:     utils.FormatResourceList(containerRecommendation.UpperBound),
-				LowerBound:     utils.FormatResourceList(containerRecommendation.LowerBound),
-				Target:         utils.FormatResourceList(containerRecommendation.Target),
-				UncappedTarget: utils.FormatResourceList(containerRecommendation.UncappedTarget),
-			}
+			var cSummary containerSummary
 			for _, c := range deployment.Spec.Template.Spec.Containers {
 				if c.Name == containerRecommendation.ContainerName {
-					klog.V(6).Infof("Resources for %s: %v", c.Name, c.Resources)
-					container.Limits = utils.FormatResourceList(c.Resources.Limits)
-					container.Requests = utils.FormatResourceList(c.Resources.Requests)
-					break
+					cSummary = containerSummary{
+						ContainerName:  containerRecommendation.ContainerName,
+						UpperBound:     utils.FormatResourceList(containerRecommendation.UpperBound),
+						LowerBound:     utils.FormatResourceList(containerRecommendation.LowerBound),
+						Target:         utils.FormatResourceList(containerRecommendation.Target),
+						UncappedTarget: utils.FormatResourceList(containerRecommendation.UncappedTarget),
+					}
+					cSummary.Limits = utils.FormatResourceList(c.Resources.Limits)
+					cSummary.Requests = utils.FormatResourceList(c.Resources.Requests)
+					klog.V(6).Infof("Resources for Deployment/%s/%s: Requests: %v Limits: %v", dSummary.DeploymentName, c.Name, cSummary.Requests, cSummary.Limits)
 				}
 			}
 
-			deploy.Containers = append(deploy.Containers, container)
+			dSummary.Containers = append(dSummary.Containers, cSummary)
 		}
-		summary.Deployments = append(summary.Deployments, deploy)
+		summary.Deployments = append(summary.Deployments, dSummary)
 	}
 
-	summary.Namespaces = utils.UniqueString(summary.Namespaces)
+	// get the unique list of namespaces we've seen for this summary
+	summary.Namespaces = summaryNamespaces.List()
+
 	return summary, nil
 }
+
+// UpdateVPAs updates the list of VPAs that the summarizer uses
+func (s *Summarizer) UpdateVPAs() error {
+	nsLog := s.namespace
+	if s.namespace == namespaceAllNamespaces {
+		nsLog = "all namespaces"
+	}
+	klog.V(3).Infof("Looking for VPAs in %s with labels: %v", nsLog, s.vpaLabels)
+	vpas, err := s.listVPAs()
+	if err != nil {
+		klog.Error(err.Error())
+		return err
+	}
+	klog.V(10).Infof("Found vpas: %v", vpas)
+
+	s.vpas = vpas
+	return nil
+}
+
+// Run creates a summary of the vpa info for all namespaces.
+func (s Summarizer) listVPAs() ([]v1beta2.VerticalPodAutoscaler, error) {
+	vpaListOptions := getVPAListOptionsForLabels(s.vpaLabels)
+	vpas, err := s.vpaClient.Client.AutoscalingV1beta2().VerticalPodAutoscalers(s.namespace).List(vpaListOptions)
+	if err != nil {
+		return nil, err
+	}
+
+	return vpas.Items, nil
+}
+
+func getVPAListOptionsForLabels(vpaLabels map[string]string) metav1.ListOptions {
+	return metav1.ListOptions{
+		LabelSelector: labels.Set(vpaLabels).String(),
+	}
+}