From 966391ba67ee51e43b480176c806b90eb60f6043 Mon Sep 17 00:00:00 2001 From: Peter Brachwitz Date: Tue, 30 Jun 2020 13:52:06 +0200 Subject: [PATCH 1/6] Add metadata for transform node role Elasticsearch 7.7.0 introduces the transform node role. Add this new node type to the pod metadata when running a version of Elasticsearch that supports transform. Adjust test cases and default values. --- .../elasticsearch/v1/elasticsearch_config.go | 35 ++++--- .../v1/elasticsearch_config_test.go | 22 +++-- pkg/apis/elasticsearch/v1/validations.go | 6 +- pkg/controller/elasticsearch/label/label.go | 14 ++- .../elasticsearch/label/label_test.go | 92 +++++++++++++++++++ .../elasticsearch/nodespec/podspec.go | 17 ++-- .../settings/canonical_config.go | 5 +- test/e2e/test/elasticsearch/checks_es.go | 10 +- test/e2e/test/elasticsearch/settings.go | 10 +- 9 files changed, 167 insertions(+), 44 deletions(-) diff --git a/pkg/apis/elasticsearch/v1/elasticsearch_config.go b/pkg/apis/elasticsearch/v1/elasticsearch_config.go index f3ff4e7953..314227d956 100644 --- a/pkg/apis/elasticsearch/v1/elasticsearch_config.go +++ b/pkg/apis/elasticsearch/v1/elasticsearch_config.go @@ -5,6 +5,7 @@ package v1 import ( + "github.com/elastic/cloud-on-k8s/pkg/controller/common/version" "github.com/elastic/go-ucfg" commonv1 "github.com/elastic/cloud-on-k8s/pkg/apis/common/v1" @@ -24,10 +25,11 @@ type ClusterSettings struct { // Node is the node section in elasticsearch.yml. type Node struct { - Master bool `config:"master"` - Data bool `config:"data"` - Ingest bool `config:"ingest"` - ML bool `config:"ml"` + Master bool `config:"master"` + Data bool `config:"data"` + Ingest bool `config:"ingest"` + ML bool `config:"ml"` + Transform bool `config:transform` // available as of 7.7.0 } // ElasticsearchSettings is a typed subset of elasticsearch.yml for purposes of the operator. @@ -37,18 +39,25 @@ type ElasticsearchSettings struct { } // DefaultCfg is an instance of ElasticsearchSettings with defaults set as they are in Elasticsearch. -var DefaultCfg = ElasticsearchSettings{ - Node: Node{ - Master: true, - Data: true, - Ingest: true, - ML: true, - }, +func DefaultCfg(ver version.Version) ElasticsearchSettings { + settings := ElasticsearchSettings{ + Node: Node{ + Master: true, + Data: true, + Ingest: true, + ML: true, + Transform: true, + }, + } + if !ver.IsSameOrAfter(version.From(7, 7, 0)) { + settings.Node.Transform = false // this setting did not exist before 7.7.0 expessed here by setting it to false + } + return settings } // Unpack unpacks Config into a typed subset. -func UnpackConfig(c *commonv1.Config) (ElasticsearchSettings, error) { - esSettings := DefaultCfg // defensive copy +func UnpackConfig(c *commonv1.Config, ver version.Version) (ElasticsearchSettings, error) { + esSettings := DefaultCfg(ver) if c == nil { // make this nil safe to allow a ptr value to work around Json serialization issues return esSettings, nil diff --git a/pkg/apis/elasticsearch/v1/elasticsearch_config_test.go b/pkg/apis/elasticsearch/v1/elasticsearch_config_test.go index 4005f54759..cb6e847601 100644 --- a/pkg/apis/elasticsearch/v1/elasticsearch_config_test.go +++ b/pkg/apis/elasticsearch/v1/elasticsearch_config_test.go @@ -8,13 +8,15 @@ import ( "testing" commonv1 "github.com/elastic/cloud-on-k8s/pkg/apis/common/v1" + "github.com/elastic/cloud-on-k8s/pkg/controller/common/version" "github.com/go-test/deep" "github.com/stretchr/testify/require" ) func TestConfig_RoleDefaults(t *testing.T) { type args struct { - c2 commonv1.Config + c2 commonv1.Config + ver version.Version } tests := []struct { name string @@ -64,9 +66,9 @@ func TestConfig_RoleDefaults(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - c1, err := UnpackConfig(&tt.c) + c1, err := UnpackConfig(&tt.c, tt.args.ver) require.NoError(t, err) - c2, err := UnpackConfig(&tt.args.c2) + c2, err := UnpackConfig(&tt.args.c2, tt.args.ver) require.NoError(t, err) if got := c1.Node == c2.Node; got != tt.want { t.Errorf("Config.EqualRoles() = %v, want %v", got, tt.want) @@ -148,6 +150,7 @@ func TestConfig_DeepCopy(t *testing.T) { } func TestConfig_Unpack(t *testing.T) { + ver := version.From(7, 7, 0) tests := []struct { name string args *commonv1.Config @@ -169,10 +172,11 @@ func TestConfig_Unpack(t *testing.T) { }, want: ElasticsearchSettings{ Node: Node{ - Master: false, - Data: true, - Ingest: true, - ML: true, + Master: false, + Data: true, + Ingest: true, + ML: true, + Transform: true, }, Cluster: ClusterSettings{ InitialMasterNodes: []string{"a", "b"}, @@ -183,13 +187,13 @@ func TestConfig_Unpack(t *testing.T) { { name: "Unpack is nil safe", args: nil, - want: DefaultCfg, + want: DefaultCfg(ver), wantErr: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := UnpackConfig(tt.args) + got, err := UnpackConfig(tt.args, ver) if (err != nil) != tt.wantErr { t.Errorf("Config.Unpack() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/pkg/apis/elasticsearch/v1/validations.go b/pkg/apis/elasticsearch/v1/validations.go index 37ee11cd19..6e8ec0826f 100644 --- a/pkg/apis/elasticsearch/v1/validations.go +++ b/pkg/apis/elasticsearch/v1/validations.go @@ -94,8 +94,12 @@ func supportedVersion(es *Elasticsearch) field.ErrorList { func hasMaster(es *Elasticsearch) field.ErrorList { var errs field.ErrorList var hasMaster bool + v, err := version.Parse(es.Spec.Version) + if err != nil { + errs = append(errs, field.Invalid(field.NewPath("spec").Child("version"), es.Spec.Version, parseVersionErrMsg)) + } for i, t := range es.Spec.NodeSets { - cfg, err := UnpackConfig(t.Config) + cfg, err := UnpackConfig(t.Config, *v) if err != nil { errs = append(errs, field.Invalid(field.NewPath("spec").Child("nodeSets").Index(i), t.Config, cfgInvalidMsg)) } diff --git a/pkg/controller/elasticsearch/label/label.go b/pkg/controller/elasticsearch/label/label.go index 852212b85f..c5d73e9b67 100644 --- a/pkg/controller/elasticsearch/label/label.go +++ b/pkg/controller/elasticsearch/label/label.go @@ -40,6 +40,8 @@ const ( NodeTypesIngestLabelName common.TrueFalseLabel = "elasticsearch.k8s.elastic.co/node-ingest" // NodeTypesMLLabelName is a label set to true on nodes with the ml role NodeTypesMLLabelName common.TrueFalseLabel = "elasticsearch.k8s.elastic.co/node-ml" + // NodeTypesTransformLabelName is a label set to true on nodes with the transform role + NodeTypesTransformLabelName common.TrueFalseLabel = "elasticsearch.k8s.elastic.co/node-transform" HTTPSchemeLabelName = "elasticsearch.k8s.elastic.co/http-scheme" @@ -112,21 +114,25 @@ func NewLabels(es types.NamespacedName) map[string]string { func NewPodLabels( es types.NamespacedName, ssetName string, - version version.Version, + ver version.Version, nodeRoles esv1.Node, configHash string, scheme string, -) (map[string]string, error) { +) map[string]string { // cluster name based labels labels := NewLabels(es) // version label - labels[VersionLabelName] = version.String() + labels[VersionLabelName] = ver.String() // node types labels NodeTypesMasterLabelName.Set(nodeRoles.Master, labels) NodeTypesDataLabelName.Set(nodeRoles.Data, labels) NodeTypesIngestLabelName.Set(nodeRoles.Ingest, labels) NodeTypesMLLabelName.Set(nodeRoles.ML, labels) + // transform nodes where only added in 7.7.0 so we should not annotate previous versions with them + if ver.IsSameOrAfter(version.From(7, 7, 0)) { + NodeTypesTransformLabelName.Set(nodeRoles.Transform, labels) + } // config hash label, to rotate pods on config changes labels[ConfigHashLabelName] = configHash @@ -138,7 +144,7 @@ func NewPodLabels( labels[k] = v } - return labels, nil + return labels } // NewConfigLabels returns labels to apply for an Elasticsearch Config secret. diff --git a/pkg/controller/elasticsearch/label/label_test.go b/pkg/controller/elasticsearch/label/label_test.go index 77c8e6ccac..3091d0a537 100644 --- a/pkg/controller/elasticsearch/label/label_test.go +++ b/pkg/controller/elasticsearch/label/label_test.go @@ -8,7 +8,10 @@ import ( "reflect" "testing" + v1 "github.com/elastic/cloud-on-k8s/pkg/apis/elasticsearch/v1" + "github.com/elastic/cloud-on-k8s/pkg/controller/common" "github.com/elastic/cloud-on-k8s/pkg/controller/common/version" + "github.com/go-test/deep" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" @@ -168,3 +171,92 @@ func TestMinVersion(t *testing.T) { }) } } + +func TestNewPodLabels(t *testing.T) { + type args struct { + es types.NamespacedName + ssetName string + ver version.Version + nodeRoles v1.Node + configHash string + scheme string + } + nameFixture := types.NamespacedName{ + Namespace: "ns", + Name: "name", + } + tests := []struct { + name string + args args + want map[string]string + wantErr bool + }{ + { + name: "labels pre-7.7", + args: args{ + es: nameFixture, + ssetName: "sset", + ver: version.From(7, 1, 0), + nodeRoles: v1.Node{ + Master: false, + Data: false, + Ingest: false, + ML: false, + Transform: false, + }, + configHash: "hash", + scheme: "https", + }, + want: map[string]string{ + ClusterNameLabelName: "name", + common.TypeLabelName: "elasticsearch", + VersionLabelName: "7.1.0", + string(NodeTypesMasterLabelName): "false", + string(NodeTypesDataLabelName): "false", + string(NodeTypesIngestLabelName): "false", + string(NodeTypesMLLabelName): "false", + ConfigHashLabelName: "hash", + HTTPSchemeLabelName: "https", + StatefulSetNameLabelName: "sset", + }, + wantErr: false, + }, + { + name: "labels post-7.7", + args: args{ + es: nameFixture, + ssetName: "sset", + ver: version.From(7, 7, 0), + nodeRoles: v1.Node{ + Master: false, + Data: true, + Ingest: false, + ML: false, + Transform: true, + }, + configHash: "hash", + scheme: "https", + }, + want: map[string]string{ + ClusterNameLabelName: "name", + common.TypeLabelName: "elasticsearch", + VersionLabelName: "7.7.0", + string(NodeTypesMasterLabelName): "false", + string(NodeTypesDataLabelName): "true", + string(NodeTypesIngestLabelName): "false", + string(NodeTypesMLLabelName): "false", + string(NodeTypesTransformLabelName): "true", + ConfigHashLabelName: "hash", + HTTPSchemeLabelName: "https", + StatefulSetNameLabelName: "sset", + }, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := NewPodLabels(tt.args.es, tt.args.ssetName, tt.args.ver, tt.args.nodeRoles, tt.args.configHash, tt.args.scheme) + require.Nil(t, deep.Equal(got, tt.want)) + }) + } +} diff --git a/pkg/controller/elasticsearch/nodespec/podspec.go b/pkg/controller/elasticsearch/nodespec/podspec.go index 96b692a48c..d087a91967 100644 --- a/pkg/controller/elasticsearch/nodespec/podspec.go +++ b/pkg/controller/elasticsearch/nodespec/podspec.go @@ -91,28 +91,25 @@ func buildLabels( nodeSet esv1.NodeSet, keystoreResources *keystore.Resources, ) (map[string]string, error) { - // label with a hash of the config to rotate the pod on config changes - unpackedCfg, err := cfg.Unpack() + // label with version + ver, err := version.Parse(es.Spec.Version) if err != nil { return nil, err } - nodeRoles := unpackedCfg.Node - cfgHash := hash.HashObject(cfg) - // label with version - ver, err := version.Parse(es.Spec.Version) + // label with a hash of the config to rotate the pod on config changes + unpackedCfg, err := cfg.Unpack(*ver) if err != nil { return nil, err } + nodeRoles := unpackedCfg.Node + cfgHash := hash.HashObject(cfg) - podLabels, err := label.NewPodLabels( + podLabels := label.NewPodLabels( k8s.ExtractNamespacedName(&es), esv1.StatefulSet(es.Name, nodeSet.Name), *ver, nodeRoles, cfgHash, es.Spec.HTTP.Protocol(), ) - if err != nil { - return nil, err - } if keystoreResources != nil { // label with a checksum of the secure settings to rotate the pod on secure settings change diff --git a/pkg/controller/elasticsearch/settings/canonical_config.go b/pkg/controller/elasticsearch/settings/canonical_config.go index a17c70c9f2..0958ecd05e 100644 --- a/pkg/controller/elasticsearch/settings/canonical_config.go +++ b/pkg/controller/elasticsearch/settings/canonical_config.go @@ -7,6 +7,7 @@ package settings import ( esv1 "github.com/elastic/cloud-on-k8s/pkg/apis/elasticsearch/v1" common "github.com/elastic/cloud-on-k8s/pkg/controller/common/settings" + "github.com/elastic/cloud-on-k8s/pkg/controller/common/version" ) // CanonicalConfig contains configuration for Elasticsearch ("elasticsearch.yml"), @@ -20,8 +21,8 @@ func NewCanonicalConfig() CanonicalConfig { } // Unpack returns a typed subset of Elasticsearch settings. -func (c CanonicalConfig) Unpack() (esv1.ElasticsearchSettings, error) { - cfg := esv1.DefaultCfg +func (c CanonicalConfig) Unpack(ver version.Version) (esv1.ElasticsearchSettings, error) { + cfg := esv1.DefaultCfg(ver) err := c.CanonicalConfig.Unpack(&cfg) return cfg, err } diff --git a/test/e2e/test/elasticsearch/checks_es.go b/test/e2e/test/elasticsearch/checks_es.go index a47c5d73bc..d50cea41c7 100644 --- a/test/e2e/test/elasticsearch/checks_es.go +++ b/test/e2e/test/elasticsearch/checks_es.go @@ -11,6 +11,7 @@ import ( "strings" esv1 "github.com/elastic/cloud-on-k8s/pkg/apis/elasticsearch/v1" + "github.com/elastic/cloud-on-k8s/pkg/controller/common/version" "github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/client" "github.com/elastic/cloud-on-k8s/test/e2e/test" "k8s.io/apimachinery/pkg/api/resource" @@ -114,6 +115,11 @@ func (e *esClusterChecks) CheckESNodesTopology(es esv1.Elasticsearch) test.Step ) } + v, err := version.Parse(es.Spec.Version) + if err != nil { + return err + } + // flatten the topology var expectedTopology []esv1.NodeSet for _, node := range es.Spec.NodeSets { @@ -148,7 +154,7 @@ func (e *esClusterChecks) CheckESNodesTopology(es esv1.Elasticsearch) test.Step nodeRoles := rolesToConfig(node.Roles) nodeStats := nodesStats.Nodes[nodeID] for i, topoElem := range expectedTopology { - cfg, err := esv1.UnpackConfig(topoElem.Config) + cfg, err := esv1.UnpackConfig(topoElem.Config, *v) if err != nil { return err } @@ -190,6 +196,8 @@ func rolesToConfig(roles []string) esv1.Node { node.Data = true case "ingest": node.Ingest = true + case "transform": + node.Transform = true } } return node diff --git a/test/e2e/test/elasticsearch/settings.go b/test/e2e/test/elasticsearch/settings.go index c55e7f6028..55353c91ea 100644 --- a/test/e2e/test/elasticsearch/settings.go +++ b/test/e2e/test/elasticsearch/settings.go @@ -7,22 +7,24 @@ package elasticsearch import ( esv1 "github.com/elastic/cloud-on-k8s/pkg/apis/elasticsearch/v1" common "github.com/elastic/cloud-on-k8s/pkg/controller/common/settings" + "github.com/elastic/cloud-on-k8s/pkg/controller/common/version" "github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/settings" ) func MustNumDataNodes(es esv1.Elasticsearch) int { var numNodes int + ver := version.MustParse(es.Spec.Version) for _, n := range es.Spec.NodeSets { - if isDataNode(n) { + if isDataNode(n, ver) { numNodes += int(n.Count) } } return numNodes } -func isDataNode(node esv1.NodeSet) bool { +func isDataNode(node esv1.NodeSet, ver version.Version) bool { if node.Config == nil { - return esv1.DefaultCfg.Node.Data // if not specified use the default + return esv1.DefaultCfg(ver).Node.Data // if not specified use the default } config, err := common.NewCanonicalConfigFrom(node.Config.Data) if err != nil { @@ -30,7 +32,7 @@ func isDataNode(node esv1.NodeSet) bool { } nodeCfg, err := settings.CanonicalConfig{ CanonicalConfig: config, - }.Unpack() + }.Unpack(ver) if err != nil { panic(err) } From ebf28919f835b5af482eeda7fb7b0ba2fbf4098d Mon Sep 17 00:00:00 2001 From: Peter Brachwitz Date: Tue, 30 Jun 2020 14:20:13 +0200 Subject: [PATCH 2/6] additional unit test --- .../elasticsearch/v1/elasticsearch_config.go | 12 ++++++---- .../v1/elasticsearch_config_test.go | 24 +++++++++++++++++++ pkg/controller/elasticsearch/label/label.go | 2 +- 3 files changed, 32 insertions(+), 6 deletions(-) diff --git a/pkg/apis/elasticsearch/v1/elasticsearch_config.go b/pkg/apis/elasticsearch/v1/elasticsearch_config.go index 314227d956..0c158cfad8 100644 --- a/pkg/apis/elasticsearch/v1/elasticsearch_config.go +++ b/pkg/apis/elasticsearch/v1/elasticsearch_config.go @@ -12,10 +12,11 @@ import ( ) const ( - NodeData = "node.data" - NodeIngest = "node.ingest" - NodeMaster = "node.master" - NodeML = "node.ml" + NodeData = "node.data" + NodeIngest = "node.ingest" + NodeMaster = "node.master" + NodeML = "node.ml" + NodeTransform = "node.transform" ) // ClusterSettings is the cluster node in elasticsearch.yml. @@ -50,7 +51,8 @@ func DefaultCfg(ver version.Version) ElasticsearchSettings { }, } if !ver.IsSameOrAfter(version.From(7, 7, 0)) { - settings.Node.Transform = false // this setting did not exist before 7.7.0 expessed here by setting it to false + // this setting did not exist before 7.7.0 expressed here by setting it to false this allows us to keep working with just one model + settings.Node.Transform = false } return settings } diff --git a/pkg/apis/elasticsearch/v1/elasticsearch_config_test.go b/pkg/apis/elasticsearch/v1/elasticsearch_config_test.go index cb6e847601..76ec6af8f9 100644 --- a/pkg/apis/elasticsearch/v1/elasticsearch_config_test.go +++ b/pkg/apis/elasticsearch/v1/elasticsearch_config_test.go @@ -63,6 +63,30 @@ func TestConfig_RoleDefaults(t *testing.T) { }, want: false, }, + { + name: "version specific default differences 1", + c: commonv1.Config{ + Data: map[string]interface{}{ + NodeTransform: true, + }, + }, + args: args{ + ver: version.From(7, 5, 0), + }, + want: false, + }, + { + name: "version specific default differences 2", + c: commonv1.Config{ + Data: map[string]interface{}{ + NodeTransform: true, + }, + }, + args: args{ + ver: version.From(7, 7, 0), + }, + want: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/controller/elasticsearch/label/label.go b/pkg/controller/elasticsearch/label/label.go index c5d73e9b67..9eaea4dd3e 100644 --- a/pkg/controller/elasticsearch/label/label.go +++ b/pkg/controller/elasticsearch/label/label.go @@ -129,7 +129,7 @@ func NewPodLabels( NodeTypesDataLabelName.Set(nodeRoles.Data, labels) NodeTypesIngestLabelName.Set(nodeRoles.Ingest, labels) NodeTypesMLLabelName.Set(nodeRoles.ML, labels) - // transform nodes where only added in 7.7.0 so we should not annotate previous versions with them + // transform nodes were only added in 7.7.0 so we should not annotate previous versions with them if ver.IsSameOrAfter(version.From(7, 7, 0)) { NodeTypesTransformLabelName.Set(nodeRoles.Transform, labels) } From 791e1194eb9c849f426e16bd15e5d6c7c92a6ca4 Mon Sep 17 00:00:00 2001 From: Peter Brachwitz Date: Tue, 30 Jun 2020 15:14:45 +0200 Subject: [PATCH 3/6] fix struct tag --- pkg/apis/elasticsearch/v1/elasticsearch_config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/apis/elasticsearch/v1/elasticsearch_config.go b/pkg/apis/elasticsearch/v1/elasticsearch_config.go index 0c158cfad8..e252dd6d72 100644 --- a/pkg/apis/elasticsearch/v1/elasticsearch_config.go +++ b/pkg/apis/elasticsearch/v1/elasticsearch_config.go @@ -30,7 +30,7 @@ type Node struct { Data bool `config:"data"` Ingest bool `config:"ingest"` ML bool `config:"ml"` - Transform bool `config:transform` // available as of 7.7.0 + Transform bool `config:"transform"` // available as of 7.7.0 } // ElasticsearchSettings is a typed subset of elasticsearch.yml for purposes of the operator. From 4e81246c2c93078ad0ea9e5ff84fbd787f02bfc3 Mon Sep 17 00:00:00 2001 From: Peter Brachwitz Date: Wed, 8 Jul 2020 15:32:17 +0200 Subject: [PATCH 4/6] Support default interdependency between data and transform --- .../elasticsearch/v1/elasticsearch_config.go | 24 ++-- .../v1/elasticsearch_config_test.go | 133 +++++++++++------- .../settings/canonical_config.go | 3 +- test/e2e/test/elasticsearch/settings.go | 3 +- 4 files changed, 106 insertions(+), 57 deletions(-) diff --git a/pkg/apis/elasticsearch/v1/elasticsearch_config.go b/pkg/apis/elasticsearch/v1/elasticsearch_config.go index e252dd6d72..6a1cbaa910 100644 --- a/pkg/apis/elasticsearch/v1/elasticsearch_config.go +++ b/pkg/apis/elasticsearch/v1/elasticsearch_config.go @@ -40,34 +40,42 @@ type ElasticsearchSettings struct { } // DefaultCfg is an instance of ElasticsearchSettings with defaults set as they are in Elasticsearch. -func DefaultCfg(ver version.Version) ElasticsearchSettings { +// cfg is the user provided config we want defaults for, ver is the version of Elasticsearch. +func DefaultCfg(cfg *ucfg.Config, ver version.Version) ElasticsearchSettings { settings := ElasticsearchSettings{ Node: Node{ Master: true, Data: true, Ingest: true, ML: true, - Transform: true, + Transform: false, }, } - if !ver.IsSameOrAfter(version.From(7, 7, 0)) { - // this setting did not exist before 7.7.0 expressed here by setting it to false this allows us to keep working with just one model - settings.Node.Transform = false + if ver.IsSameOrAfter(version.From(7, 7, 0)) { + // this setting did not exist before 7.7.0 its default depends on the node.data value + settings.Node.Transform = true + if cfg == nil { + return settings + } + dataNode, err := cfg.Bool(NodeData, -1, commonv1.CfgOptions...) + if err == nil && !dataNode { + settings.Node.Transform = false + } } return settings } // Unpack unpacks Config into a typed subset. func UnpackConfig(c *commonv1.Config, ver version.Version) (ElasticsearchSettings, error) { - esSettings := DefaultCfg(ver) if c == nil { // make this nil safe to allow a ptr value to work around Json serialization issues - return esSettings, nil + return DefaultCfg(ucfg.New(), ver), nil } config, err := ucfg.NewFrom(c.Data, commonv1.CfgOptions...) if err != nil { - return esSettings, err + return ElasticsearchSettings{}, err } + esSettings := DefaultCfg(config, ver) err = config.Unpack(&esSettings, commonv1.CfgOptions...) return esSettings, err } diff --git a/pkg/apis/elasticsearch/v1/elasticsearch_config_test.go b/pkg/apis/elasticsearch/v1/elasticsearch_config_test.go index 76ec6af8f9..34337f6029 100644 --- a/pkg/apis/elasticsearch/v1/elasticsearch_config_test.go +++ b/pkg/apis/elasticsearch/v1/elasticsearch_config_test.go @@ -15,87 +15,118 @@ import ( func TestConfig_RoleDefaults(t *testing.T) { type args struct { - c2 commonv1.Config + c commonv1.Config ver version.Version } tests := []struct { - name string - c commonv1.Config - args args - want bool + name string + args args + wantCfg Node }{ { - name: "empty is equal", - c: commonv1.Config{}, + name: "empty equals defaults", args: args{}, - want: true, + wantCfg: Node{ + Master: true, + Data: true, + Ingest: true, + ML: true, + Transform: false, + }, }, { - name: "same is equal", - c: commonv1.Config{ - Data: map[string]interface{}{ - NodeMaster: true, - }, - }, + name: "same as default", args: args{ - c2: commonv1.Config{ + c: commonv1.Config{ Data: map[string]interface{}{ NodeMaster: true, }, }, }, - want: true, + wantCfg: Node{ + Master: true, + Data: true, + Ingest: true, + ML: true, + Transform: false, + }, }, { - name: "detect differences", - c: commonv1.Config{ - Data: map[string]interface{}{ - NodeMaster: false, - NodeData: true, - }, - }, + name: "differ from default", args: args{ - c2: commonv1.Config{ + c: commonv1.Config{ Data: map[string]interface{}{ - NodeData: true, + NodeData: false, }, }, }, - want: false, + wantCfg: Node{ + Master: true, + Data: false, + Ingest: true, + ML: true, + Transform: false, + }, }, { - name: "version specific default differences 1", - c: commonv1.Config{ - Data: map[string]interface{}{ - NodeTransform: true, - }, - }, + name: "version specific default differences: transform", args: args{ - ver: version.From(7, 5, 0), + ver: version.From(7, 7, 0), + }, + wantCfg: Node{ + Master: true, + Data: true, + Ingest: true, + ML: true, + Transform: true, }, - want: false, }, { - name: "version specific default differences 2", - c: commonv1.Config{ - Data: map[string]interface{}{ - NodeTransform: true, + name: "transform data interdependency", + args: args{ + c: commonv1.Config{ + Data: map[string]interface{}{ + "node": map[string]interface{}{ + "data": false, + }, + }, }, + ver: version.From(7, 7, 0), + }, + wantCfg: Node{ + Master: true, + Data: false, + Ingest: true, + ML: true, + Transform: false, }, + }, + { + name: "transform data interdependency", args: args{ + c: commonv1.Config{ + Data: map[string]interface{}{ + NodeData: false, + NodeTransform: true, + }, + }, ver: version.From(7, 7, 0), }, - want: true, + wantCfg: Node{ + Master: true, + Data: false, + Ingest: true, + ML: true, + Transform: true, + }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - c1, err := UnpackConfig(&tt.c, tt.args.ver) + got, err := UnpackConfig(&tt.args.c, tt.args.ver) require.NoError(t, err) - c2, err := UnpackConfig(&tt.args.c2, tt.args.ver) - require.NoError(t, err) - if got := c1.Node == c2.Node; got != tt.want { - t.Errorf("Config.EqualRoles() = %v, want %v", got, tt.want) + if got.Node != tt.wantCfg { + t.Errorf("Config.EqualRoles() = %+v, want %+v", got, tt.wantCfg) } }) } @@ -209,9 +240,17 @@ func TestConfig_Unpack(t *testing.T) { wantErr: false, }, { - name: "Unpack is nil safe", - args: nil, - want: DefaultCfg(ver), + name: "Unpack is nil safe", + args: nil, + want: ElasticsearchSettings{ + Node: Node{ + Master: true, + Data: true, + Ingest: true, + ML: true, + Transform: true, + }, + }, wantErr: false, }, } diff --git a/pkg/controller/elasticsearch/settings/canonical_config.go b/pkg/controller/elasticsearch/settings/canonical_config.go index 0958ecd05e..04021ad83b 100644 --- a/pkg/controller/elasticsearch/settings/canonical_config.go +++ b/pkg/controller/elasticsearch/settings/canonical_config.go @@ -8,6 +8,7 @@ import ( esv1 "github.com/elastic/cloud-on-k8s/pkg/apis/elasticsearch/v1" common "github.com/elastic/cloud-on-k8s/pkg/controller/common/settings" "github.com/elastic/cloud-on-k8s/pkg/controller/common/version" + "github.com/elastic/go-ucfg" ) // CanonicalConfig contains configuration for Elasticsearch ("elasticsearch.yml"), @@ -22,7 +23,7 @@ func NewCanonicalConfig() CanonicalConfig { // Unpack returns a typed subset of Elasticsearch settings. func (c CanonicalConfig) Unpack(ver version.Version) (esv1.ElasticsearchSettings, error) { - cfg := esv1.DefaultCfg(ver) + cfg := esv1.DefaultCfg((*ucfg.Config)(c.CanonicalConfig), ver) err := c.CanonicalConfig.Unpack(&cfg) return cfg, err } diff --git a/test/e2e/test/elasticsearch/settings.go b/test/e2e/test/elasticsearch/settings.go index 55353c91ea..8798e58e50 100644 --- a/test/e2e/test/elasticsearch/settings.go +++ b/test/e2e/test/elasticsearch/settings.go @@ -9,6 +9,7 @@ import ( common "github.com/elastic/cloud-on-k8s/pkg/controller/common/settings" "github.com/elastic/cloud-on-k8s/pkg/controller/common/version" "github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/settings" + "github.com/elastic/go-ucfg" ) func MustNumDataNodes(es esv1.Elasticsearch) int { @@ -24,7 +25,7 @@ func MustNumDataNodes(es esv1.Elasticsearch) int { func isDataNode(node esv1.NodeSet, ver version.Version) bool { if node.Config == nil { - return esv1.DefaultCfg(ver).Node.Data // if not specified use the default + return esv1.DefaultCfg(ucfg.New(), ver).Node.Data // if not specified use the default } config, err := common.NewCanonicalConfigFrom(node.Config.Data) if err != nil { From f198ae438d3562c6618075560a124eb16f4f65a2 Mon Sep 17 00:00:00 2001 From: Peter Brachwitz Date: Tue, 21 Jul 2020 17:40:33 +0200 Subject: [PATCH 5/6] Mention node.transform in docs --- .../elasticsearch/advanced-node-scheduling.asciidoc | 2 ++ .../elasticsearch/node-configuration.asciidoc | 2 ++ 2 files changed, 4 insertions(+) diff --git a/docs/orchestrating-elastic-stack-applications/elasticsearch/advanced-node-scheduling.asciidoc b/docs/orchestrating-elastic-stack-applications/elasticsearch/advanced-node-scheduling.asciidoc index 61b511fc6e..7fcf119883 100644 --- a/docs/orchestrating-elastic-stack-applications/elasticsearch/advanced-node-scheduling.asciidoc +++ b/docs/orchestrating-elastic-stack-applications/elasticsearch/advanced-node-scheduling.asciidoc @@ -38,6 +38,8 @@ spec: node.master: true node.data: false node.ingest: false + node.ml: false + node.transform: false node.remote_cluster_client: false # 3 ingest-data nodes - name: ingest-data diff --git a/docs/orchestrating-elastic-stack-applications/elasticsearch/node-configuration.asciidoc b/docs/orchestrating-elastic-stack-applications/elasticsearch/node-configuration.asciidoc index 0bda9e1957..2146c6e48e 100644 --- a/docs/orchestrating-elastic-stack-applications/elasticsearch/node-configuration.asciidoc +++ b/docs/orchestrating-elastic-stack-applications/elasticsearch/node-configuration.asciidoc @@ -22,6 +22,7 @@ spec: node.ingest: false node.ml: false xpack.ml.enabled: true + node.transform: false node.remote_cluster_client: false - name: data count: 10 @@ -30,6 +31,7 @@ spec: node.data: true node.ingest: true node.ml: true + node.transform: true node.remote_cluster_client: false ---- From 6601335e78665beb922fa9fcb684eb3ce3885ac8 Mon Sep 17 00:00:00 2001 From: Peter Brachwitz Date: Wed, 22 Jul 2020 12:22:36 +0200 Subject: [PATCH 6/6] Update pkg/apis/elasticsearch/v1/elasticsearch_config.go Co-authored-by: Thibault Richard --- pkg/apis/elasticsearch/v1/elasticsearch_config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/apis/elasticsearch/v1/elasticsearch_config.go b/pkg/apis/elasticsearch/v1/elasticsearch_config.go index c7cf4d19d7..411a162cfd 100644 --- a/pkg/apis/elasticsearch/v1/elasticsearch_config.go +++ b/pkg/apis/elasticsearch/v1/elasticsearch_config.go @@ -109,7 +109,7 @@ func DefaultCfg(cfg *ucfg.Config, ver version.Version) ElasticsearchSettings { return settings } -// Unpack unpacks Config into a typed subset. +// UnpackConfig unpacks Config into a typed subset. func UnpackConfig(c *commonv1.Config, ver version.Version) (ElasticsearchSettings, error) { if c == nil { // make this nil safe to allow a ptr value to work around Json serialization issues