From af6c3ff4c5d19ff92aefe9cf8578a483e09fa5d1 Mon Sep 17 00:00:00 2001 From: wuqixuan Date: Mon, 6 Jul 2015 16:52:12 +0800 Subject: [PATCH 1/6] fleetd: support operators in metadata If define metadata in fleet conf, such as "ram=1024", we can define the operator in [X-Fleet] unit as below: [X-Fleet] MachineMetadata=ram>=2048 The operators have been supported: "<=", ">=", "!=", "<", ">", "=" If the operatior are "<=", ">=", "!=", "<", ">", the value should be integer, otherwise, the unit will never be launched. Fixes #1143 --- job/job.go | 12 ++++++- machine/machine.go | 80 ++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 89 insertions(+), 3 deletions(-) diff --git a/job/job.go b/job/job.go index fc5723dc4..42b4ac50c 100644 --- a/job/job.go +++ b/job/job.go @@ -284,7 +284,17 @@ func (j *Job) RequiredTargetMetadata() map[string]pkg.Set { fleetMachineMetadata, } { for _, valuePair := range j.requirements()[key] { - s := strings.Split(valuePair, "=") + var s []string + for _, sep := range []string{"<=", ">=", "!=", "<", ">"} { + index := strings.Index(valuePair, sep) + if index != -1 { + s = []string{valuePair[0:index], valuePair[index:]} + break + } + } + if s == nil { + s = strings.Split(valuePair, "=") + } if len(s) != 2 { continue diff --git a/machine/machine.go b/machine/machine.go index 8be43d5a1..fc67c9a95 100644 --- a/machine/machine.go +++ b/machine/machine.go @@ -15,6 +15,9 @@ package machine import ( + "strconv" + "strings" + "github.com/coreos/fleet/log" "github.com/coreos/fleet/pkg" ) @@ -38,8 +41,81 @@ func HasMetadata(state *MachineState, metadata map[string]pkg.Set) bool { if values.Contains(local) { log.Debugf("Local Metadata(%s) meets requirement", key) } else { - log.Debugf("Local Metadata(%s) does not match requirement", key) - return false + vs := values.Values() + for _, v := range vs { + if index := strings.Index(v, "<="); strings.Contains(v, "<=") && (index == 0) { + need, err1 := strconv.Atoi(v[2:]) + have, err2 := strconv.Atoi(local) + if err1 == nil && err2 == nil { + if have <= need { + log.Debugf("Local Metadata(%s) meets requirement", key) + continue + } else { + log.Debugf("Local Metadata(%s) does not match requirement", key) + return false + } + } else { + log.Debugf("Local Metadata(%s) does not match requirement", key) + return false + } + } else if index := strings.Index(v, ">="); strings.Contains(v, ">=") && (index == 0) { + need, err1 := strconv.Atoi(v[2:]) + have, err2 := strconv.Atoi(local) + if err1 == nil && err2 == nil { + if have >= need { + log.Debugf("Local Metadata(%s) meets requirement", key) + continue + } else { + log.Debugf("Local Metadata(%s) does not match requirement", key) + return false + } + } else { + log.Debugf("Local Metadata(%s) does not match requirement", key) + return false + } + } else if index := strings.Index(v, ">"); strings.Contains(v, ">") && (index == 0) { + need, err1 := strconv.Atoi(v[1:]) + have, err2 := strconv.Atoi(local) + if err1 == nil && err2 == nil { + if have > need { + log.Debugf("Local Metadata(%s) meets requirement", key) + continue + } else { + log.Debugf("Local Metadata(%s) does not match requirement", key) + return false + } + } else { + log.Debugf("Local Metadata(%s) does not match requirement", key) + return false + } + } else if index := strings.Index(v, "<"); strings.Contains(v, "<") && (index == 0) { + need, err1 := strconv.Atoi(v[1:]) + have, err2 := strconv.Atoi(local) + if err1 == nil && err2 == nil { + if have < need { + log.Debugf("Local Metadata(%s) meets requirement", key) + continue + } else { + log.Debugf("Local Metadata(%s) does not match requirement", key) + return false + } + } else { + log.Debugf("Local Metadata(%s) does not match requirement", key) + return false + } + } else if index := strings.Index(v, "!="); strings.Contains(v, "!=") && (index == 0) { + if v[2:] != local { + log.Debugf("Local Metadata(%s) meets requirement", key) + continue + } else { + log.Debugf("Local Metadata(%s) does not match requirement", key) + return false + } + } else { + log.Debugf("Local Metadata(%s) does not match requirement", key) + return false + } + } } } From 2c76ec0ac8e581766abab87f40a5d6a4d1d4ed76 Mon Sep 17 00:00:00 2001 From: Dongsu Park Date: Fri, 3 Jun 2016 17:26:03 +0200 Subject: [PATCH 2/6] fleetd: support also "==" operator for metadata To follow the maintainer's suggestion, also support the operator "==", which is actually the same as "=". --- job/job.go | 2 +- machine/machine.go | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/job/job.go b/job/job.go index 42b4ac50c..c4dec6f7c 100644 --- a/job/job.go +++ b/job/job.go @@ -285,7 +285,7 @@ func (j *Job) RequiredTargetMetadata() map[string]pkg.Set { } { for _, valuePair := range j.requirements()[key] { var s []string - for _, sep := range []string{"<=", ">=", "!=", "<", ">"} { + for _, sep := range []string{"==", "<=", ">=", "!=", "<", ">"} { index := strings.Index(valuePair, sep) if index != -1 { s = []string{valuePair[0:index], valuePair[index:]} diff --git a/machine/machine.go b/machine/machine.go index fc67c9a95..bb7b7182e 100644 --- a/machine/machine.go +++ b/machine/machine.go @@ -111,6 +111,14 @@ func HasMetadata(state *MachineState, metadata map[string]pkg.Set) bool { log.Debugf("Local Metadata(%s) does not match requirement", key) return false } + } else if index := strings.Index(v, "=="); strings.Contains(v, "==") && (index == 0) { + if v[2:] == local { + log.Debugf("Local Metadata(%s) meets requirement", key) + continue + } else { + log.Debugf("Local Metadata(%s) does not match requirement", key) + return false + } } else { log.Debugf("Local Metadata(%s) does not match requirement", key) return false From 669c083da25e8858785a303941a8041878fd2068 Mon Sep 17 00:00:00 2001 From: wuqixuan Date: Tue, 7 Jul 2015 16:44:39 +0800 Subject: [PATCH 3/6] Documentation: describe about operators in metadata Update documentation. Fixes #1143 --- Documentation/unit-files-and-scheduling.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Documentation/unit-files-and-scheduling.md b/Documentation/unit-files-and-scheduling.md index badaf5e32..d37f58cb7 100644 --- a/Documentation/unit-files-and-scheduling.md +++ b/Documentation/unit-files-and-scheduling.md @@ -179,6 +179,14 @@ app.service 282f949f.../10.10.20.1 active running app.service fd1d3e94.../10.0.0.1 active running ``` +`MachineMetadata` also support relational operators, including `<=`, `>=`, `<`, `>`, `==` and `!=`: + +``` +[X-Fleet] +MachineMetadata=ram<1024 +``` +This requires an eligible machine to have the `ram` less than 1024. The value must be numeral when using `<=`, `>=`, `<` or `>`. + A machine is not automatically configured with metadata. A deployer may define machine metadata using the `metadata` [config option][config-option]. From a110b4592404fefbbab85865abe3fa528c2d475e Mon Sep 17 00:00:00 2001 From: Dongsu Park Date: Fri, 3 Jun 2016 17:39:14 +0200 Subject: [PATCH 4/6] machine: add more unit tests for metadata operators TestHasMetadata should also test more cases for metadata operators. --- machine/machine_test.go | 114 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 114 insertions(+) diff --git a/machine/machine_test.go b/machine/machine_test.go index 8cb7d2c09..910e23f72 100644 --- a/machine/machine_test.go +++ b/machine/machine_test.go @@ -71,6 +71,120 @@ func TestHasMetadata(t *testing.T) { }, false, }, + // operator '<=' + { + map[string]string{ + "ram": "1024", + }, + map[string]pkg.Set{ + "ram": pkg.NewUnsafeSet("<=1024"), + }, + true, + }, + { + map[string]string{ + "ram": "1025", + }, + map[string]pkg.Set{ + "ram": pkg.NewUnsafeSet("<=1024"), + }, + false, + }, + // operator '>=' + { + map[string]string{ + "ram": "1024", + }, + map[string]pkg.Set{ + "ram": pkg.NewUnsafeSet(">=1024"), + }, + true, + }, + { + map[string]string{ + "ram": "1023", + }, + map[string]pkg.Set{ + "ram": pkg.NewUnsafeSet(">=1024"), + }, + false, + }, + // operator '!=' + { + map[string]string{ + "ram": "1024", + }, + map[string]pkg.Set{ + "ram": pkg.NewUnsafeSet("!=1024"), + }, + false, + }, + { + map[string]string{ + "ram": "1023", + }, + map[string]pkg.Set{ + "ram": pkg.NewUnsafeSet("!=1024"), + }, + true, + }, + // operator '<' + { + map[string]string{ + "ram": "1023", + }, + map[string]pkg.Set{ + "ram": pkg.NewUnsafeSet("<1024"), + }, + true, + }, + { + map[string]string{ + "ram": "1024", + }, + map[string]pkg.Set{ + "ram": pkg.NewUnsafeSet("<1024"), + }, + false, + }, + // operator '>' + { + map[string]string{ + "ram": "1025", + }, + map[string]pkg.Set{ + "ram": pkg.NewUnsafeSet(">1024"), + }, + true, + }, + { + map[string]string{ + "ram": "1024", + }, + map[string]pkg.Set{ + "ram": pkg.NewUnsafeSet(">1024"), + }, + false, + }, + // operator '==' + { + map[string]string{ + "ram": "1025", + }, + map[string]pkg.Set{ + "ram": pkg.NewUnsafeSet("==1024"), + }, + false, + }, + { + map[string]string{ + "ram": "1024", + }, + map[string]pkg.Set{ + "ram": pkg.NewUnsafeSet("==1024"), + }, + true, + }, } for i, tt := range testCases { From 05eba5b36528474e517146d61ce57c85f820269e Mon Sep 17 00:00:00 2001 From: Dongsu Park Date: Wed, 13 Jul 2016 15:11:28 +0200 Subject: [PATCH 5/6] fleetctl: make list-machines print out metadata operators fleetctl list-machines should distinguish normal metadata from metadata with operators, to print out human-readable messages. --- fleetctl/list_machines.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/fleetctl/list_machines.go b/fleetctl/list_machines.go index 2d8ebbded..dd9ff4c03 100644 --- a/fleetctl/list_machines.go +++ b/fleetctl/list_machines.go @@ -123,7 +123,11 @@ func formatMetadata(metadata map[string]string) string { sorted.Sort() for _, key := range sorted { value := metadata[key] - pairs[idx] = fmt.Sprintf("%s=%s", key, value) + if hasMetadataOperator(value) { + pairs[idx] = fmt.Sprintf("%s%s", key, value) + } else { + pairs[idx] = fmt.Sprintf("%s=%s", key, value) + } idx++ } return strings.Join(pairs, ",") @@ -135,3 +139,12 @@ func machineToFieldKeys(m map[string]machineToField) (keys []string) { } return } + +func hasMetadataOperator(instr string) bool { + for _, op := range []string{"<=", ">=", "!=", "==", "<", ">"} { + if strings.HasPrefix(instr, op) { + return true + } + } + return false +} From 68028c7e5dcae80013124ff34a3794452770ec58 Mon Sep 17 00:00:00 2001 From: Dongsu Park Date: Thu, 14 Jul 2016 13:00:30 +0200 Subject: [PATCH 6/6] functional: introduce a test TestMetadataOperator TestMetadataOperator ensures that metadata operators work also for extended operators such as ">=", "<=", "<", ">", "!=", or "==". First make the test machine have "ram=1024" in its machine metadata. Then in TestMetadataOperator, check each possible operator one after another, to make sure that each works without error. --- functional/fixtures/units/metadata-op.service | 5 ++ functional/metadata_test.go | 85 +++++++++++++++++++ functional/util/config.go | 5 +- 3 files changed, 94 insertions(+), 1 deletion(-) create mode 100644 functional/fixtures/units/metadata-op.service diff --git a/functional/fixtures/units/metadata-op.service b/functional/fixtures/units/metadata-op.service new file mode 100644 index 000000000..fd28c8a63 --- /dev/null +++ b/functional/fixtures/units/metadata-op.service @@ -0,0 +1,5 @@ +[Service] +ExecStart=/bin/bash -c "while true; do echo Hello, World!; sleep 1; done" + +[X-Fleet] +MachineMetadata="ram>=1024" diff --git a/functional/metadata_test.go b/functional/metadata_test.go index 5308abf8e..7a9b2345c 100644 --- a/functional/metadata_test.go +++ b/functional/metadata_test.go @@ -16,6 +16,7 @@ package functional import ( "fmt" + "path" "regexp" "strings" "testing" @@ -90,3 +91,87 @@ func TestTemplatesWithSpecifiersInMetadata(t *testing.T) { t.Fatalf("metadata@invalid unit should not be scheduled: \nstdout: %s\nstderr: %s", stdout, stderr) } } + +// TestMetadataOperator ensures that metadata operators work also for +// extended operators such as ">=", "<=", "<", ">", "!=", or "==". +func TestMetadataOperator(t *testing.T) { + cluster, err := platform.NewNspawnCluster("smoke") + if err != nil { + t.Fatal(err) + } + defer cluster.Destroy(t) + + members, err := platform.CreateNClusterMembers(cluster, 1) + if err != nil { + t.Fatal(err) + } + m0 := members[0] + _, err = cluster.WaitForNMachines(m0, 1) + if err != nil { + t.Fatal(err) + } + + stdout, stderr, err := cluster.Fleetctl(m0, "list-machines", "--fields", "machine,metadata") + if err != nil { + t.Fatalf("Unable to get machine metadata\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err) + } + + runMetaOp := func(ramEq string, expectSuccess bool) { + tmpMdOpService := "/tmp/metadata-op.service" + MdOpService := "fixtures/units/metadata-op.service" + MdOpBaseName := path.Base(MdOpService) + var nUnits int + + if expectSuccess { + t.Logf("Testing %s expecting success...", ramEq) + nUnits = 1 + } else { + t.Logf("Testing %s expecting failure...", ramEq) + nUnits = 0 + } + + err = util.GenNewFleetService(tmpMdOpService, MdOpService, ramEq, "ram>=1024") + if err != nil { + t.Fatalf("Failed to generate a temp fleet service: %v", err) + } + + stdout, stderr, err = cluster.Fleetctl(m0, "start", "--no-block", tmpMdOpService) + if err != nil { + t.Fatalf("starting unit %s returned error:\nstdout: %s\nstderr: %s\nerr: %v", + tmpMdOpService, stdout, stderr, err) + } + + _, err = cluster.WaitForNActiveUnits(m0, nUnits) + if err != nil { + t.Fatal(err) + } + + stdout, stderr, err = cluster.Fleetctl(m0, "destroy", MdOpBaseName) + if err != nil { + t.Fatalf("unit %s cannot be stopped: \nstdout: %s\nstderr: %s\nerr: %v", + MdOpBaseName, stdout, stderr, err) + } + + _, err = cluster.WaitForNUnitFiles(m0, 0) + if err != nil { + t.Fatal(err) + } + + } + + // run tests for success cases + runMetaOp("ram>=1024", true) + runMetaOp("ram<=1024", true) + runMetaOp("ram>1023", true) + runMetaOp("ram<1025", true) + runMetaOp("ram!=1025", true) + runMetaOp("ram==1024", true) + + // run tests for failure cases + runMetaOp("ram>=1025", false) + runMetaOp("ram<=1023", false) + runMetaOp("ram>1024", false) + runMetaOp("ram<1024", false) + runMetaOp("ram!=1024", false) + runMetaOp("ram==1025", false) +} diff --git a/functional/util/config.go b/functional/util/config.go index 66d94c431..c91b35c83 100644 --- a/functional/util/config.go +++ b/functional/util/config.go @@ -26,6 +26,7 @@ import ( const ( fleetAPIPort = 54728 + fleetRAM = 1024 // in MiB FleetTTL = "3s" cloudConfig = `#cloud-config @@ -62,7 +63,7 @@ coreos: command: start content: | [Service] - Environment=FLEET_METADATA=hostname=%H + Environment=FLEET_METADATA=hostname=%H,ram={{printf "%d" .FleetRAM}} ExecStart=/opt/fleet/fleetd -config /opt/fleet/fleet.conf ` ) @@ -80,6 +81,7 @@ type configValues struct { EtcdEndpoint string EtcdKeyPrefix string FleetAPIPort int + FleetRAM int FleetAgentTTL string FleetExtra string } @@ -118,6 +120,7 @@ func BuildCloudConfig(dst io.Writer, ip, etcdEndpoint, etcdKeyPrefix string) err EtcdEndpoint: etcdEndpoint, EtcdKeyPrefix: etcdKeyPrefix, FleetAPIPort: fleetAPIPort, + FleetRAM: fleetRAM, FleetAgentTTL: FleetTTL, FleetExtra: os.Getenv("FLEETD_TEST_ENV"), }