Skip to content

Commit

Permalink
Refactor:
Browse files Browse the repository at this point in the history
- migrate `SortManifests` and its test case to `releaseutil`
- replace the `sortByKind` in `releaseutil`
- migrate `Manifest`, `ManifestFile`, `Result` types to `releaseutil`
- migrate the `Sort` mathod of `ManifestFile` to `releaseutil`
- remove trivial code in `tiller` package.

Signed-off-by: Lentil1016 <lentil1016@gmail.com>
  • Loading branch information
lentil1016 committed Jan 30, 2019
1 parent 86d8596 commit b578ece
Show file tree
Hide file tree
Showing 13 changed files with 53 additions and 775 deletions.
7 changes: 3 additions & 4 deletions cmd/helm/template.go
Expand Up @@ -36,7 +36,6 @@ import (
"k8s.io/helm/pkg/engine"
"k8s.io/helm/pkg/hapi/release"
util "k8s.io/helm/pkg/releaseutil"
"k8s.io/helm/pkg/tiller"
)

const defaultDirectoryPermission = 0755
Expand Down Expand Up @@ -194,7 +193,7 @@ func (o *templateOptions) run(out io.Writer) error {
return err
}

listManifests := []tiller.Manifest{}
listManifests := []util.Manifest{}
// extract kind and name
re := regexp.MustCompile("kind:(.*)\n")
for k, v := range rendered {
Expand All @@ -203,7 +202,7 @@ func (o *templateOptions) run(out io.Writer) error {
if len(match) == 2 {
h = strings.TrimSpace(match[1])
}
m := tiller.Manifest{Name: k, Content: v, Head: &util.SimpleHead{Kind: h}}
m := util.Manifest{Name: k, Content: v, Head: &util.SimpleHead{Kind: h}}
listManifests = append(listManifests, m)
}
in := func(needle string, haystack []string) bool {
Expand Down Expand Up @@ -231,7 +230,7 @@ func (o *templateOptions) run(out io.Writer) error {
printRelease(out, rel)
}

for _, m := range tiller.SortByKind(listManifests) {
for _, m := range util.SortByKind(listManifests, util.InstallOrder) {
b := filepath.Base(m.Name)
switch {
case len(o.renderFiles) > 0 && !in(m.Name, rf):
Expand Down
1 change: 0 additions & 1 deletion pkg/action/install.go
Expand Up @@ -293,7 +293,6 @@ func (i *Install) renderResources(ch *chart.Chart, values chartutil.Values, vs c
// Sort hooks, manifests, and partials. Only hooks and manifests are returned,
// as partials are not used after renderer.Render. Empty manifests are also
// removed here.
// TODO: Can we migrate SortManifests out of pkg/tiller?
hooks, manifests, err := releaseutil.SortManifests(files, vs, releaseutil.InstallOrder)
if err != nil {
// By catching parse errors here, we can prevent bogus releases from going
Expand Down
12 changes: 2 additions & 10 deletions pkg/releaseutil/kind_sorter.go
Expand Up @@ -83,10 +83,10 @@ var UninstallOrder KindSortOrder = []string{
"Namespace",
}

// sortByKind does an in-place sort of manifests by Kind.
// SortByKind does an in-place sort of manifests by Kind.
//
// Results are sorted by 'ordering'
func sortByKind(manifests []Manifest, ordering KindSortOrder) []Manifest {
func SortByKind(manifests []Manifest, ordering KindSortOrder) []Manifest {
ks := newKindSorter(manifests, ordering)
sort.Sort(ks)
return ks.manifests
Expand Down Expand Up @@ -136,11 +136,3 @@ func (k *kindSorter) Less(i, j int) bool {
// sort different kinds
return first < second
}

// SortByKind sorts manifests in InstallOrder
func SortByKind(manifests []Manifest) []Manifest {
ordering := InstallOrder
ks := newKindSorter(manifests, ordering)
sort.Sort(ks)
return ks.manifests
}
4 changes: 2 additions & 2 deletions pkg/releaseutil/kind_sorter_test.go
Expand Up @@ -143,7 +143,7 @@ func TestKindSorter(t *testing.T) {
t.Fatalf("Expected %d names in order, got %d", want, got)
}
defer buf.Reset()
for _, r := range sortByKind(manifests, test.order) {
for _, r := range SortByKind(manifests, test.order) {
buf.WriteString(r.Name)
}
if got := buf.String(); got != test.expected {
Expand Down Expand Up @@ -204,7 +204,7 @@ func TestKindSorterSubSort(t *testing.T) {
var buf bytes.Buffer
t.Run(test.description, func(t *testing.T) {
defer buf.Reset()
for _, r := range sortByKind(manifests, test.order) {
for _, r := range SortByKind(manifests, test.order) {
buf.WriteString(r.Name)
}
if got := buf.String(); got != test.expected {
Expand Down
58 changes: 29 additions & 29 deletions pkg/releaseutil/manifest_sorter.go
Expand Up @@ -36,17 +36,17 @@ type Manifest struct {
Head *SimpleHead
}

// manifestFile represents a file that contains a manifest.
type manifestFile struct {
entries map[string]string
path string
apis chartutil.VersionSet
// ManifestFile represents a file that contains a manifest.
type ManifestFile struct {
Entries map[string]string
Path string
APIs chartutil.VersionSet
}

// result is an intermediate structure used during sorting.
type result struct {
hooks []*release.Hook
generic []Manifest
// Result is an intermediate structure used during sorting.
type Result struct {
Hooks []*release.Hook
Generic []Manifest
}

// TODO: Refactor this out. It's here because naming conventions were not followed through.
Expand Down Expand Up @@ -74,7 +74,7 @@ var events = map[string]release.HookEvent{
// Files that do not parse into the expected format are simply placed into a map and
// returned.
func SortManifests(files map[string]string, apis chartutil.VersionSet, sort KindSortOrder) ([]*release.Hook, []Manifest, error) {
result := &result{}
result := &Result{}

for filePath, c := range files {

Expand All @@ -89,21 +89,21 @@ func SortManifests(files map[string]string, apis chartutil.VersionSet, sort Kind
continue
}

manifestFile := &manifestFile{
entries: SplitManifests(c),
path: filePath,
apis: apis,
manifestFile := &ManifestFile{
Entries: SplitManifests(c),
Path: filePath,
APIs: apis,
}

if err := manifestFile.sort(result); err != nil {
return result.hooks, result.generic, err
if err := manifestFile.Sort(result); err != nil {
return result.Hooks, result.Generic, err
}
}

return result.hooks, sortByKind(result.generic, sort), nil
return result.Hooks, SortByKind(result.Generic, sort), nil
}

// sort takes a manifestFile object which may contain multiple resource definition
// Sort takes a ManifestFile object which may contain multiple resource definition
// entries and sorts each entry by hook types, and saves the resulting hooks and
// generic manifests (or non-hooks) to the result struct.
//
Expand All @@ -122,20 +122,20 @@ func SortManifests(files map[string]string, apis chartutil.VersionSet, sort Kind
// metadata:
// annotations:
// helm.sh/hook-delete-policy: hook-succeeded
func (file *manifestFile) sort(result *result) error {
for _, m := range file.entries {
func (file *ManifestFile) Sort(result *Result) error {
for _, m := range file.Entries {
var entry SimpleHead
if err := yaml.Unmarshal([]byte(m), &entry); err != nil {
return errors.Wrapf(err, "YAML parse error on %s", file.path)
return errors.Wrapf(err, "YAML parse error on %s", file.Path)
}

if entry.Version != "" && !file.apis.Has(entry.Version) {
return errors.Errorf("apiVersion %q in %s is not available", entry.Version, file.path)
if entry.Version != "" && !file.APIs.Has(entry.Version) {
return errors.Errorf("apiVersion %q in %s is not available", entry.Version, file.Path)
}

if !hasAnyAnnotation(entry) {
result.generic = append(result.generic, Manifest{
Name: file.path,
result.Generic = append(result.Generic, Manifest{
Name: file.Path,
Content: m,
Head: &entry,
})
Expand All @@ -144,8 +144,8 @@ func (file *manifestFile) sort(result *result) error {

hookTypes, ok := entry.Metadata.Annotations[hooks.HookAnno]
if !ok {
result.generic = append(result.generic, Manifest{
Name: file.path,
result.Generic = append(result.Generic, Manifest{
Name: file.Path,
Content: m,
Head: &entry,
})
Expand All @@ -157,7 +157,7 @@ func (file *manifestFile) sort(result *result) error {
h := &release.Hook{
Name: entry.Metadata.Name,
Kind: entry.Kind,
Path: file.path,
Path: file.Path,
Manifest: m,
Events: []release.HookEvent{},
Weight: hw,
Expand All @@ -180,7 +180,7 @@ func (file *manifestFile) sort(result *result) error {
continue
}

result.hooks = append(result.hooks, h)
result.Hooks = append(result.Hooks, h)

operateAnnotationValues(entry, hooks.HookDeleteAnno, func(value string) {
h.DeletePolicies = append(h.DeletePolicies, release.HookDeletePolicy(value))
Expand Down
2 changes: 1 addition & 1 deletion pkg/releaseutil/manifest_sorter_test.go
Expand Up @@ -220,7 +220,7 @@ metadata:
}
}

sorted = sortByKind(sorted, InstallOrder)
sorted = SortByKind(sorted, InstallOrder)
for i, m := range generic {
if m.Content != sorted[i].Content {
t.Errorf("Expected %q, got %q", m.Content, sorted[i].Content)
Expand Down

0 comments on commit b578ece

Please sign in to comment.