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.

Sign up for GitHub

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

Remove unused Tags and FSRoot from NodeUp #9737

Merged
merged 5 commits into from
Aug 13, 2020
Merged

Remove unused Tags and FSRoot from NodeUp #9737

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d75042c
Remove unused Tags from NodeUp
Aug 12, 2020
44db702
Update bazel
Aug 12, 2020
ec8bb51
Update packages
Aug 12, 2020
d70fb50
Remove unused FSRoot from NodeUp
Aug 12, 2020
b913e35
Remove also the flagRootFS flag from NodeUp
Aug 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions cmd/nodeup/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const (
func main() {
klog.InitFlags(nil)

var flagConf, flagCacheDir, flagRootFS, gitVersion string
var flagConf, flagCacheDir, gitVersion string
var flagRetries int
var dryrun, installSystemdUnit bool
target := "direct"
Expand All @@ -47,7 +47,6 @@ func main() {
fmt.Printf("nodeup version %s%s\n", kops.Version, gitVersion)
flag.StringVar(&flagConf, "conf", "node.yaml", "configuration location")
flag.StringVar(&flagCacheDir, "cache", "/var/cache/nodeup", "the location for the local asset cache")
flag.StringVar(&flagRootFS, "rootfs", "/", "the location of the machine root (for running in a container)")
flag.IntVar(&flagRetries, "retries", -1, "maximum number of retries on failure: -1 means retry forever")
flag.BoolVar(&dryrun, "dryrun", false, "Don't create cloud resources; just show what would be done")
flag.StringVar(&target, "target", target, "Target - direct, cloudinit")
Expand Down Expand Up @@ -101,7 +100,6 @@ func main() {
i := bootstrap.Installation{
CacheDir: flagCacheDir,
Command: command,
FSRoot: flagRootFS,
hakman marked this conversation as resolved.
Show resolved Hide resolved
}
i.RunTasksOptions.InitDefaults()
i.RunTasksOptions.MaxTaskDuration = 5 * time.Minute
Expand All @@ -115,7 +113,6 @@ func main() {
ConfigLocation: flagConf,
Target: target,
CacheDir: flagCacheDir,
FSRoot: flagRootFS,
}
err = cmd.Run(os.Stdout)
if err == nil {
Expand Down
1 change: 0 additions & 1 deletion hack/.packages
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,6 @@ k8s.io/kops/upup/pkg/fi/nodeup
k8s.io/kops/upup/pkg/fi/nodeup/cloudinit
k8s.io/kops/upup/pkg/fi/nodeup/local
k8s.io/kops/upup/pkg/fi/nodeup/nodetasks
k8s.io/kops/upup/pkg/fi/nodeup/tags
k8s.io/kops/upup/pkg/fi/secrets
k8s.io/kops/upup/pkg/fi/utils
k8s.io/kops/upup/pkg/kutil
Expand Down
1 change: 0 additions & 1 deletion nodeup/pkg/bootstrap/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ go_library(
"//upup/pkg/fi/nodeup/local:go_default_library",
"//upup/pkg/fi/nodeup/nodetasks:go_default_library",
"//util/pkg/vfs:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
"//vendor/k8s.io/klog:go_default_library",
],
)
8 changes: 1 addition & 7 deletions nodeup/pkg/bootstrap/install.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"os"
"strings"

"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/klog"
"k8s.io/kops/nodeup/pkg/distros"
"k8s.io/kops/pkg/systemd"
Expand All @@ -33,21 +32,17 @@ import (
)

type Installation struct {
FSRoot string
CacheDir string
RunTasksOptions fi.RunTasksOptions
Command []string
}

func (i *Installation) Run() error {
distribution, err := distros.FindDistribution(i.FSRoot)
_, err := distros.FindDistribution("/")
Copy link
Member

Choose a reason for hiding this comment

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

I think we could get rid of the string argument in FindDistribution() since it is only ever passed "/"

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe we want to add some tests for it in the future, or use in some container?

if err != nil {
return fmt.Errorf("error determining OS distribution: %v", err)
}

tags := sets.NewString()
tags.Insert(distribution.BuildTags()...)

tasks := make(map[string]fi.Task)

buildContext := &fi.ModelBuilderContext{
Expand Down Expand Up @@ -75,7 +70,6 @@ func (i *Installation) Run() error {

target := &local.LocalTarget{
CacheDir: i.CacheDir,
Tags: tags,
}

checkExisting := true
Expand Down
5 changes: 1 addition & 4 deletions nodeup/pkg/distros/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,5 @@ go_library(
],
importpath = "k8s.io/kops/nodeup/pkg/distros",
visibility = ["//visibility:public"],
deps = [
"//upup/pkg/fi/nodeup/tags:go_default_library",
"//vendor/k8s.io/klog:go_default_library",
],
deps = ["//vendor/k8s.io/klog:go_default_library"],
)
45 changes: 0 additions & 45 deletions nodeup/pkg/distros/distribution.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package distros

import (
"k8s.io/klog"
"k8s.io/kops/upup/pkg/fi/nodeup/tags"
)

type Distribution string
Expand All @@ -38,50 +37,6 @@ var (
DistributionContainerOS Distribution = "containeros"
)

func (d Distribution) BuildTags() []string {
var t []string

switch d {
case DistributionDebian9, DistributionDebian10:
t = []string{} // trying to move away from tags
case DistributionXenial:
t = []string{"_xenial"}
case DistributionBionic:
t = []string{"_bionic"}
case DistributionFocal:
t = []string{"_focal"}
case DistributionAmazonLinux2:
t = []string{"_amazonlinux2"}
case DistributionCentos7:
t = []string{"_centos7"}
case DistributionRhel7:
t = []string{"_rhel7"}
case DistributionCentos8:
t = []string{"_centos8"}
case DistributionRhel8:
t = []string{"_rhel8"}
case DistributionFlatcar:
t = []string{"_flatcar"}
case DistributionContainerOS:
t = []string{"_containeros"}
default:
klog.Fatalf("unknown distribution: %s", d)
return nil
}

if d.IsDebianFamily() {
t = append(t, tags.TagOSFamilyDebian)
}
if d.IsRHELFamily() {
t = append(t, tags.TagOSFamilyRHEL)
}
if d.IsSystemd() {
t = append(t, tags.TagSystemd)
}

return t
}

func (d Distribution) IsDebianFamily() bool {
switch d {
case DistributionDebian9, DistributionDebian10:
Expand Down
1 change: 0 additions & 1 deletion upup/pkg/fi/nodeup/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ go_library(
"//vendor/github.com/aws/aws-sdk-go/aws:go_default_library",
"//vendor/github.com/aws/aws-sdk-go/aws/session:go_default_library",
"//vendor/github.com/aws/aws-sdk-go/service/ec2:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
"//vendor/k8s.io/klog:go_default_library",
],
)
1 change: 0 additions & 1 deletion upup/pkg/fi/nodeup/cloudinit/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ go_library(
deps = [
"//upup/pkg/fi:go_default_library",
"//upup/pkg/fi/utils:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
"//vendor/k8s.io/klog:go_default_library",
],
)
10 changes: 1 addition & 9 deletions upup/pkg/fi/nodeup/cloudinit/cloud_init_target.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"os"
"path"

"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/klog"
"k8s.io/kops/upup/pkg/fi"
"k8s.io/kops/upup/pkg/fi/utils"
Expand All @@ -32,7 +31,6 @@ import (
type CloudInitTarget struct {
Config *CloudConfig
out io.Writer
Tags sets.String
}

type AddBehaviour int
Expand All @@ -42,11 +40,10 @@ const (
Once
)

func NewCloudInitTarget(out io.Writer, tags sets.String) *CloudInitTarget {
func NewCloudInitTarget(out io.Writer) *CloudInitTarget {
t := &CloudInitTarget{
Config: &CloudConfig{},
out: out,
Tags: tags,
}
return t
}
Expand All @@ -69,11 +66,6 @@ type CloudConfigFile struct {
Content string `json:"content,omitempty"`
}

func (t *CloudInitTarget) HasTag(tag string) bool {
_, found := t.Tags[tag]
return found
}

func (t *CloudInitTarget) ProcessDeletions() bool {
// We don't expect any, but it would be our job to process them
return true
Expand Down
22 changes: 2 additions & 20 deletions upup/pkg/fi/nodeup/command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ import (
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/session"
"github.com/aws/aws-sdk-go/service/ec2"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/klog"
)

Expand All @@ -57,7 +56,6 @@ const MaxTaskDuration = 365 * 24 * time.Hour
type NodeUpCommand struct {
CacheDir string
ConfigLocation string
FSRoot string
Target string
cluster *api.Cluster
config *nodeup.Config
Expand All @@ -66,10 +64,6 @@ type NodeUpCommand struct {

// Run is responsible for perform the nodeup process
func (c *NodeUpCommand) Run(out io.Writer) error {
if c.FSRoot == "" {
return fmt.Errorf("FSRoot is required")
}

if c.ConfigLocation != "" {
config, err := vfs.Context.ReadFile(c.ConfigLocation)
if err != nil {
Expand Down Expand Up @@ -163,22 +157,11 @@ func (c *NodeUpCommand) Run(out io.Writer) error {
return fmt.Errorf("error determining OS architecture: %v", err)
}

archTags := architecture.BuildTags()

distribution, err := distros.FindDistribution(c.FSRoot)
distribution, err := distros.FindDistribution("/")
if err != nil {
return fmt.Errorf("error determining OS distribution: %v", err)
}

distroTags := distribution.BuildTags()

nodeTags := sets.NewString()
nodeTags.Insert(archTags...)
nodeTags.Insert(distroTags...)

klog.Infof("Arch tags: %v", archTags)
klog.Infof("Distro tags: %v", distroTags)

configAssets := c.config.Assets[architecture]
assetStore := fi.NewAssetStore(c.CacheDir)
for _, asset := range configAssets {
Expand Down Expand Up @@ -290,14 +273,13 @@ func (c *NodeUpCommand) Run(out io.Writer) error {
case "direct":
target = &local.LocalTarget{
CacheDir: c.CacheDir,
Tags: nodeTags,
}
case "dryrun":
assetBuilder := assets.NewAssetBuilder(c.cluster, "")
target = fi.NewDryRunTarget(assetBuilder, out)
case "cloudinit":
checkExisting = false
target = cloudinit.NewCloudInitTarget(out, nodeTags)
target = cloudinit.NewCloudInitTarget(out)
default:
return fmt.Errorf("unsupported target type %q", c.Target)
}
Expand Down
5 changes: 1 addition & 4 deletions upup/pkg/fi/nodeup/local/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,5 @@ go_library(
srcs = ["local_target.go"],
importpath = "k8s.io/kops/upup/pkg/fi/nodeup/local",
visibility = ["//visibility:public"],
deps = [
"//upup/pkg/fi:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
],
deps = ["//upup/pkg/fi:go_default_library"],
)
7 changes: 0 additions & 7 deletions upup/pkg/fi/nodeup/local/local_target.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,11 @@ package local
import (
"os/exec"

"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/kops/upup/pkg/fi"
)

type LocalTarget struct {
CacheDir string
Tags sets.String
}

var _ fi.Target = &LocalTarget{}
Expand All @@ -39,11 +37,6 @@ func (t *LocalTarget) ProcessDeletions() bool {
return true
}

func (t *LocalTarget) HasTag(tag string) bool {
_, found := t.Tags[tag]
return found
}

// CombinedOutput is a helper function that executes a command, returning stdout & stderr combined
func (t *LocalTarget) CombinedOutput(args []string) ([]byte, error) {
c := exec.Command(args[0], args[1:]...)
Expand Down
2 changes: 1 addition & 1 deletion upup/pkg/fi/nodeup/nodetasks/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@ go_library(
importpath = "k8s.io/kops/upup/pkg/fi/nodeup/nodetasks",
visibility = ["//visibility:public"],
deps = [
"//nodeup/pkg/distros:go_default_library",
"//pkg/apis/kops:go_default_library",
"//pkg/backoff:go_default_library",
"//pkg/kubeconfig:go_default_library",
"//pkg/pki:go_default_library",
"//upup/pkg/fi:go_default_library",
"//upup/pkg/fi/nodeup/cloudinit:go_default_library",
"//upup/pkg/fi/nodeup/local:go_default_library",
"//upup/pkg/fi/nodeup/tags:go_default_library",
"//upup/pkg/fi/utils:go_default_library",
"//util/pkg/hashing:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
Expand Down
Loading