From ba469fbf7f1af1ebd18a050dcea5fd972e5cf00c Mon Sep 17 00:00:00 2001 From: jackgr Date: Mon, 28 Mar 2016 05:06:56 -0700 Subject: [PATCH] Refactor expander --- cmd/manager/manager/expander.go | 172 +++++++++--------------- cmd/manager/manager/expander_test.go | 189 ++++++++++++++------------- 2 files changed, 161 insertions(+), 200 deletions(-) diff --git a/cmd/manager/manager/expander.go b/cmd/manager/manager/expander.go index a8b2d1fa0..9dda285e7 100644 --- a/cmd/manager/manager/expander.go +++ b/cmd/manager/manager/expander.go @@ -17,65 +17,53 @@ limitations under the License. package manager import ( + "github.com/kubernetes/helm/pkg/common" + "github.com/kubernetes/helm/pkg/expansion" + "github.com/kubernetes/helm/pkg/repo" + "bytes" "encoding/json" "fmt" "io/ioutil" "net/http" - - "github.com/ghodss/yaml" - "github.com/kubernetes/helm/pkg/common" ) +/* const ( // TODO (iantw): Align this with a character not allowed to show up in resource names. layoutNodeKeySeparator = "#" ) +*/ -// ExpandedTemplate is the structure returned by the expansion service. -type ExpandedTemplate struct { +// ExpandedConfiguration is the structure returned by the expansion service. +type ExpandedConfiguration struct { Config *common.Configuration `json:"config"` Layout *common.Layout `json:"layout"` } // Expander abstracts interactions with the expander and deployer services. type Expander interface { - ExpandTemplate(t *common.Template) (*ExpandedTemplate, error) -} - -// TODO: Remove mockResolver when type resolver is completely excised -type mockResolver struct { -} - -func (r *mockResolver) ResolveTypes(c *common.Configuration, i []*common.ImportFile) ([]*common.ImportFile, error) { - return nil, nil + ExpandConfiguration(conf *common.Configuration) (*ExpandedConfiguration, error) } // NewExpander returns a new initialized Expander. -func NewExpander(url string) Expander { - tr := &mockResolver{} - return &expander{url, tr} +func NewExpander(URL string, rp repo.IRepoProvider) Expander { + if rp == nil { + rp = repo.NewRepoProvider(nil, nil, nil) + } + + return &expander{expanderURL: URL, repoProvider: rp} } type expander struct { + repoProvider repo.IRepoProvider expanderURL string - typeResolver TypeResolver -} - -// TypeResolver finds Types in a Configuration which aren't yet reduceable to an import file -// or primitive, and attempts to replace them with a template from a URL. -type TypeResolver interface { - ResolveTypes(config *common.Configuration, imports []*common.ImportFile) ([]*common.ImportFile, error) } func (e *expander) getBaseURL() string { return fmt.Sprintf("%s/expand", e.expanderURL) } -func expanderError(t *common.Template, err error) error { - return fmt.Errorf("cannot expand template named %s (%s):\n%s", t.Name, err, t.Content) -} - // ExpanderResponse gives back a layout, which has nested structure // Resource0 // ResourceDefinition @@ -108,6 +96,7 @@ func expanderError(t *common.Template, err error) error { // between the name#template key to exist in the layout given a particular choice of naming. // In practice, it would be nearly impossible to hit, but consider including properties/name/type // into a hash of sorts to make this robust... +/* func walkLayout(l *common.Layout, imports []*common.ImportFile, toReplace map[string]*common.LayoutResource) map[string]*common.LayoutResource { ret := map[string]*common.LayoutResource{} toVisit := l.Resources @@ -126,81 +115,68 @@ func walkLayout(l *common.Layout, imports []*common.ImportFile, toReplace map[st return ret } +*/ -// isTemplate returns whether a given type is a template. -func isTemplate(t string, imports []*common.ImportFile) bool { - for _, imp := range imports { - if imp.Name == t { - return true - } +// ExpandConfiguration expands the supplied configuration and returns +// an expanded configuration. +func (e *expander) ExpandConfiguration(conf *common.Configuration) (*ExpandedConfiguration, error) { + expConf, err := e.expandConfiguration(conf) + if err != nil { + return nil, fmt.Errorf("cannot expand configuration:%s\n%v\n", err, conf) } - return false + return expConf, nil } -// ExpandTemplate expands the supplied template, and returns a configuration. -// It will also update the imports in the provided template if any were added -// during type resolution. -func (e *expander) ExpandTemplate(t *common.Template) (*ExpandedTemplate, error) { - // We have a fencepost problem here. - // 1. Start by trying to resolve any missing templates - // 2. Expand the configuration using all the of the imports available to us at this point - // 3. Expansion may yield additional templates, so we run the type resolution again - // 4. If type resolution resulted in new imports being available, return to 2. - config := &common.Configuration{} - if err := yaml.Unmarshal([]byte(t.Content), config); err != nil { - e := fmt.Errorf("Unable to unmarshal configuration (%s): %s", err, t.Content) - return nil, e - } - - var finalLayout *common.Layout - needResolve := map[string]*common.LayoutResource{} +func (e *expander) expandConfiguration(conf *common.Configuration) (*ExpandedConfiguration, error) { + resources := []*common.Resource{} + layout := []*common.LayoutResource{} - // Start things off by attempting to resolve the templates in a first pass. - newImp, err := e.typeResolver.ResolveTypes(config, t.Imports) - if err != nil { - e := fmt.Errorf("type resolution failed: %s", err) - return nil, expanderError(t, e) - } + for _, resource := range conf.Resources { + if !repo.IsChartReference(resource.Type) { + resources = append(resources, resource) + continue + } - t.Imports = append(t.Imports, newImp...) + cbr, _, err := e.repoProvider.GetChartByReference(resource.Type) + if err != nil { + return nil, err + } - for { - // Now expand with everything imported. - result, err := e.expandTemplate(t) + defer cbr.Close() + content, err := cbr.LoadContent() if err != nil { - e := fmt.Errorf("template expansion: %s", err) - return nil, expanderError(t, e) + return nil, err } - // Once we set this layout, we're operating on the "needResolve" *LayoutResources, - // which are pointers into the original layout structure. After each expansion we - // lose the templates in the previous expansion, so we have to keep the first one - // around and keep appending to the pointers in it as we get more layers of expansion. - if finalLayout == nil { - finalLayout = result.Layout + svcReq := &expansion.ServiceRequest{ + ChartInvocation: resource, + Chart: content, } - needResolve = walkLayout(result.Layout, t.Imports, needResolve) - newImp, err = e.typeResolver.ResolveTypes(result.Config, t.Imports) + svcResp, err := e.callService(svcReq) if err != nil { - e := fmt.Errorf("type resolution failed: %s", err) - return nil, expanderError(t, e) + return nil, err } - // If the new imports contain nothing, we are done. Everything is fully expanded. - if len(newImp) == 0 { - result.Layout = finalLayout - return result, nil + expConf, err := e.expandConfiguration(svcResp) + if err != nil { + return nil, err } - // Update imports with any new imports from type resolution. - t.Imports = append(t.Imports, newImp...) + // TODO: build up layout hiearchically + resources = append(resources, expConf.Config.Resources...) + layout = append(layout, expConf.Layout.Resources...) } + + return &ExpandedConfiguration{ + Config: &common.Configuration{Resources: resources}, + Layout: &common.Layout{Resources: layout}, + }, nil } -func (e *expander) expandTemplate(t *common.Template) (*ExpandedTemplate, error) { - j, err := json.Marshal(t) +func (e *expander) callService(svcReq *expansion.ServiceRequest) (*common.Configuration, error) { + j, err := json.Marshal(svcReq) if err != nil { return nil, err } @@ -232,37 +208,11 @@ func (e *expander) expandTemplate(t *common.Template) (*ExpandedTemplate, error) return nil, err } - er := &ExpansionResponse{} - if err := json.Unmarshal(body, er); err != nil { + svcResp := &common.Configuration{} + if err := json.Unmarshal(body, svcResp); err != nil { e := fmt.Errorf("cannot unmarshal response body (%s):%s", err, body) return nil, e } - template, err := er.Unmarshal() - if err != nil { - e := fmt.Errorf("cannot unmarshal response yaml (%s):%v", err, er) - return nil, e - } - - return template, nil -} - -// ExpansionResponse describes the results of marshaling an ExpandedTemplate. -type ExpansionResponse struct { - Config string `json:"config"` - Layout string `json:"layout"` -} - -// Unmarshal creates and returns an ExpandedTemplate from an ExpansionResponse. -func (er *ExpansionResponse) Unmarshal() (*ExpandedTemplate, error) { - template := &ExpandedTemplate{} - if err := yaml.Unmarshal([]byte(er.Config), &template.Config); err != nil { - return nil, fmt.Errorf("cannot unmarshal config (%s):\n%s", err, er.Config) - } - - if err := yaml.Unmarshal([]byte(er.Layout), &template.Layout); err != nil { - return nil, fmt.Errorf("cannot unmarshal layout (%s):\n%s", err, er.Layout) - } - - return template, nil + return svcResp, nil } diff --git a/cmd/manager/manager/expander_test.go b/cmd/manager/manager/expander_test.go index f374c0176..e79ca8f78 100644 --- a/cmd/manager/manager/expander_test.go +++ b/cmd/manager/manager/expander_test.go @@ -26,44 +26,24 @@ import ( "strings" "testing" + "github.com/ghodss/yaml" + "github.com/kubernetes/helm/pkg/chart" "github.com/kubernetes/helm/pkg/common" + "github.com/kubernetes/helm/pkg/expansion" + "github.com/kubernetes/helm/pkg/repo" "github.com/kubernetes/helm/pkg/util" - - "github.com/ghodss/yaml" ) -var validTemplateTestCaseData = common.Template{ - Name: "TestTemplate", - Content: string(validContentTestCaseData), - Imports: validImportFilesTestCaseData, -} - -var validContentTestCaseData = []byte(` -imports: -- path: test-type.py -resources: -- name: test - type: test-type.py - properties: - test-property: test-value -`) - -var validImportFilesTestCaseData = []*common.ImportFile{ - { - Name: "test-type.py", - Content: "test-type.py validTemplateTestCaseData content", - }, - { - Name: "test.py", - Content: "test.py validTemplateTestCaseData content", - }, - { - Name: "test2.py", - Content: "test2.py validTemplateTestCaseData content", - }, -} +var ( + TestRepoBucket = "kubernetes-charts-testing" + TestRepoURL = "gs://" + TestRepoBucket + TestChartName = "frobnitz" + TestChartVersion = "0.0.1" + TestArchiveName = TestChartName + "-" + TestChartVersion + ".tgz" + TestResourceType = TestRepoURL + "/" + TestArchiveName +) -var validConfigTestCaseData = []byte(` +var validResponseTestCaseData = []byte(` resources: - name: test-service properties: @@ -91,6 +71,7 @@ resources: type: ReplicationController `) +/* var validLayoutTestCaseData = []byte(` resources: - name: test @@ -126,18 +107,12 @@ resources: type: test2.jinja `) -var validResponseTestCaseData = ExpansionResponse{ - Config: string(validConfigTestCaseData), - Layout: string(validLayoutTestCaseData), -} - var roundTripContent = ` -config: - resources: - - name: test - type: test.py - properties: - test: test +resources: +- name: test + type: test.py + properties: + test: test ` var roundTripExpanded = ` @@ -207,35 +182,58 @@ layout: test: test ` -var roundTripTemplate = common.Template{ - Name: "TestTemplate", - Content: roundTripContent, - Imports: nil, +var roundTripResponse = &ExpandedConfiguration{ + Config: roundTripExpanded, +} + +var roundTripResponse2 = &ExpandedConfiguration{ + Config: roundTripExpanded2, +} + +var roundTripResponses = []*ExpandedConfiguration{ + roundTripResponse, + roundTripResponse2, +} +*/ + +type mockRepoProvider struct { +} + +func (m *mockRepoProvider) GetChartByReference(reference string) (*chart.Chart, repo.IChartRepo, error) { + return &chart.Chart{}, nil, nil +} + +func (m *mockRepoProvider) GetRepoByChartURL(URL string) (repo.IChartRepo, error) { + return nil, nil +} + +func (m *mockRepoProvider) GetRepoByURL(URL string) (repo.IChartRepo, error) { + return nil, nil } type ExpanderTestCase struct { Description string Error string Handler func(w http.ResponseWriter, r *http.Request) - ValidResponse *ExpandedTemplate + ValidResponse *ExpandedConfiguration } func TestExpandTemplate(t *testing.T) { - roundTripResponse := &ExpandedTemplate{} - if err := yaml.Unmarshal([]byte(finalExpanded), roundTripResponse); err != nil { - panic(err) - } + // roundTripResponse := &ExpandedConfiguration{} + // if err := yaml.Unmarshal([]byte(finalExpanded), roundTripResponse); err != nil { + // panic(err) + // } tests := []ExpanderTestCase{ { - "expect success for ExpandTemplate", + "expect success for ExpandConfiguration", "", expanderSuccessHandler, - getValidResponse(t, "expect success for ExpandTemplate"), + getValidExpandedConfiguration(), }, { - "expect error for ExpandTemplate", - "cannot expand template", + "expect error for ExpandConfiguration", + "simulated failure", expanderErrorHandler, nil, }, @@ -245,12 +243,23 @@ func TestExpandTemplate(t *testing.T) { ts := httptest.NewServer(http.HandlerFunc(etc.Handler)) defer ts.Close() - expander := NewExpander(ts.URL) - actualResponse, err := expander.ExpandTemplate(&validTemplateTestCaseData) + expander := NewExpander(ts.URL, nil) + resource := &common.Resource{ + Name: "test_invocation", + Type: TestResourceType, + } + + conf := &common.Configuration{ + Resources: []*common.Resource{ + resource, + }, + } + + actualResponse, err := expander.ExpandConfiguration(conf) if err != nil { message := err.Error() if etc.Error == "" { - t.Errorf("Error in test case %s when there should not be.", etc.Description) + t.Errorf("unexpected error in test case %s: %s", etc.Description, err) } if !strings.Contains(message, etc.Error) { t.Errorf("error in test case:%s:%s\n", etc.Description, message) @@ -270,42 +279,41 @@ func TestExpandTemplate(t *testing.T) { } } -func getValidResponse(t *testing.T, description string) *ExpandedTemplate { - response, err := validResponseTestCaseData.Unmarshal() - if err != nil { - t.Errorf("cannot unmarshal valid response for test case '%s': %s\n", description, err) +func getValidServiceResponse() *common.Configuration { + conf := &common.Configuration{} + if err := yaml.Unmarshal(validResponseTestCaseData, conf); err != nil { + panic(fmt.Errorf("cannot unmarshal valid response: %s\n", err)) } - return response + return conf } -func expanderErrorHandler(w http.ResponseWriter, r *http.Request) { - defer r.Body.Close() - http.Error(w, "something failed", http.StatusInternalServerError) -} +func getValidExpandedConfiguration() *ExpandedConfiguration { + conf := getValidServiceResponse() + layout := &common.Layout{Resources: []*common.LayoutResource{}} + return &ExpandedConfiguration{Config: conf, Layout: layout} -var roundTripResponse = ExpansionResponse{ - Config: roundTripExpanded, - Layout: roundTripLayout, } -var roundTripResponse2 = ExpansionResponse{ - Config: roundTripExpanded2, - Layout: roundTripLayout2, -} - -var roundTripResponses = []ExpansionResponse{ - roundTripResponse, - roundTripResponse2, +func expanderErrorHandler(w http.ResponseWriter, r *http.Request) { + defer r.Body.Close() + http.Error(w, "simulated failure", http.StatusInternalServerError) } +/* func roundTripHandler(w http.ResponseWriter, r *http.Request) { defer r.Body.Close() handler := "expandybird: expand" util.LogHandlerEntry(handler, r) + if len(roundTripResponses) < 1 { + http.Error(w, "Too many calls to round trip handler", http.StatusInternalServerError) + return + } + util.LogHandlerExitWithJSON(handler, w, roundTripResponses[0], http.StatusOK) roundTripResponses = roundTripResponses[1:] } +*/ func expanderSuccessHandler(w http.ResponseWriter, r *http.Request) { handler := "expandybird: expand" @@ -318,19 +326,22 @@ func expanderSuccessHandler(w http.ResponseWriter, r *http.Request) { return } - template := &common.Template{} - if err := json.Unmarshal(body, template); err != nil { + svcReq := &expansion.ServiceRequest{} + if err := json.Unmarshal(body, svcReq); err != nil { status := fmt.Sprintf("cannot unmarshal request body:%s\n%s\n", err, body) http.Error(w, status, http.StatusInternalServerError) return } - if !reflect.DeepEqual(validTemplateTestCaseData, *template) { - status := fmt.Sprintf("error in http handler:\nwant:%s\nhave:%s\n", - util.ToJSONOrError(validTemplateTestCaseData), util.ToJSONOrError(template)) - http.Error(w, status, http.StatusInternalServerError) - return - } + /* + if !reflect.DeepEqual(validRequestTestCaseData, *svcReq) { + status := fmt.Sprintf("error in http handler:\nwant:%s\nhave:%s\n", + util.ToJSONOrError(validRequestTestCaseData), util.ToJSONOrError(template)) + http.Error(w, status, http.StatusInternalServerError) + return + } + */ - util.LogHandlerExitWithJSON(handler, w, validResponseTestCaseData, http.StatusOK) + svcResp := getValidServiceResponse() + util.LogHandlerExitWithJSON(handler, w, svcResp, http.StatusOK) }