Skip to content

Commit

Permalink
Merge pull request #4909 from r-vasquez/remove-json-config
Browse files Browse the repository at this point in the history
rpk: remove readAsJson from config manager
  • Loading branch information
twmb committed May 24, 2022
2 parents 341cb88 + 74a67ef commit 4b4ec2c
Show file tree
Hide file tree
Showing 9 changed files with 28 additions and 155 deletions.
1 change: 0 additions & 1 deletion src/go/k8s/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ require (
github.com/hashicorp/errwrap v1.0.0 // indirect
github.com/hashicorp/golang-lru v0.5.4 // indirect
github.com/hashicorp/hcl v1.0.0 // indirect
github.com/icza/dyno v0.0.0-20200205103839-49cb13720835 // indirect
github.com/imdario/mergo v0.3.12 // indirect
github.com/inconshreveable/mousetrap v1.0.0 // indirect
github.com/magiconair/properties v1.8.1 // indirect
Expand Down
2 changes: 0 additions & 2 deletions src/go/k8s/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -381,8 +381,6 @@ github.com/hinshun/vt10x v0.0.0-20180616224451-1954e6464174/go.mod h1:DqJ97dSdRW
github.com/howeyc/gopass v0.0.0-20170109162249-bf9dde6d0d2c/go.mod h1:lADxMC39cJJqL93Duh1xhAs4I2Zs8mKS89XWXFGp9cs=
github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU=
github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc=
github.com/icza/dyno v0.0.0-20200205103839-49cb13720835 h1:f1irK5f03uGGj+FjgQfZ5VhdKNVQVJ4skHsedzVohQ4=
github.com/icza/dyno v0.0.0-20200205103839-49cb13720835/go.mod h1:c1tRKs5Tx7E2+uHGSyyncziFjvGpgv4H2HrqXeUQ/Uk=
github.com/imdario/mergo v0.3.5/go.mod h1:2EnlNZ0deacrJVfApfmtdGgDfMuh/nq6Ok1EcJh5FfA=
github.com/imdario/mergo v0.3.9/go.mod h1:2EnlNZ0deacrJVfApfmtdGgDfMuh/nq6Ok1EcJh5FfA=
github.com/imdario/mergo v0.3.12 h1:b6R2BslTbIEToALKP7LxUvijTsNI9TAe80pLWN2g/HU=
Expand Down
1 change: 0 additions & 1 deletion src/go/rpk/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ require (
github.com/fatih/color v1.7.0
github.com/google/uuid v1.1.1
github.com/hashicorp/go-multierror v1.1.0
github.com/icza/dyno v0.0.0-20200205103839-49cb13720835
github.com/lorenzosaino/go-sysctl v0.1.0
github.com/mitchellh/mapstructure v1.4.1
github.com/olekukonko/tablewriter v0.0.1
Expand Down
2 changes: 0 additions & 2 deletions src/go/rpk/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,6 @@ github.com/hashicorp/memberlist v0.1.3/go.mod h1:ajVTdAv/9Im8oMAAj5G31PhhMCZJV2p
github.com/hashicorp/serf v0.8.2/go.mod h1:6hOLApaqBFA1NXqRQAsxw9QxuDEvNxSQRwA/JwenrHc=
github.com/hinshun/vt10x v0.0.0-20180616224451-1954e6464174 h1:WlZsjVhE8Af9IcZDGgJGQpNflI3+MJSBhsgT5PCtzBQ=
github.com/hinshun/vt10x v0.0.0-20180616224451-1954e6464174/go.mod h1:DqJ97dSdRW1W22yXSB90986pcOyQ7r45iio1KN2ez1A=
github.com/icza/dyno v0.0.0-20200205103839-49cb13720835 h1:f1irK5f03uGGj+FjgQfZ5VhdKNVQVJ4skHsedzVohQ4=
github.com/icza/dyno v0.0.0-20200205103839-49cb13720835/go.mod h1:c1tRKs5Tx7E2+uHGSyyncziFjvGpgv4H2HrqXeUQ/Uk=
github.com/inconshreveable/mousetrap v1.0.0 h1:Z8tu5sraLXCXIcARxBp/8cbvlwVa7Z1NHg9XEKhtSvM=
github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8=
github.com/jcmturner/aescts/v2 v2.0.0/go.mod h1:AiaICIRyfYg35RUkr8yESTqvSy7csK90qZ5xfvvsoNs=
Expand Down
34 changes: 13 additions & 21 deletions src/go/rpk/pkg/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,20 +70,19 @@ type metricsBody struct {
}

type environmentBody struct {
Payload EnvironmentPayload `json:"payload"`
Config map[string]interface{} `json:"config"`
SentAt time.Time `json:"sentAt"`
NodeUUID string `json:"nodeUuid"`
Organization string `json:"organization"`
ClusterID string `json:"clusterId"`
NodeID int `json:"nodeId"`
CloudVendor string `json:"cloudVendor"`
VMType string `json:"vmType"`
OSInfo string `json:"osInfo"`
CPUModel string `json:"cpuModel"`
CPUCores int `json:"cpuCores"`
RPVersion string `json:"rpVersion"`
Environment string `json:"environment"`
Payload EnvironmentPayload `json:"payload"`
SentAt time.Time `json:"sentAt"`
NodeUUID string `json:"nodeUuid"`
Organization string `json:"organization"`
ClusterID string `json:"clusterId"`
NodeID int `json:"nodeId"`
CloudVendor string `json:"cloudVendor"`
VMType string `json:"vmType"`
OSInfo string `json:"osInfo"`
CPUModel string `json:"cpuModel"`
CPUCores int `json:"cpuCores"`
RPVersion string `json:"rpVersion"`
Environment string `json:"environment"`
}

func SendMetrics(p MetricsPayload, conf config.Config) error {
Expand All @@ -102,14 +101,8 @@ func SendEnvironment(
fs afero.Fs,
env EnvironmentPayload,
conf config.Config,
confJSON string,
skipCloudCheck bool,
) error {
confMap := map[string]interface{}{}
err := json.Unmarshal([]byte(confJSON), &confMap)
if err != nil {
return err
}
cloudVendor := na
vmType := na
if !skipCloudCheck {
Expand Down Expand Up @@ -146,7 +139,6 @@ func SendEnvironment(

b := environmentBody{
Payload: env,
Config: confMap,
SentAt: time.Now(),
NodeUUID: conf.NodeUUID,
Organization: conf.Organization,
Expand Down
18 changes: 0 additions & 18 deletions src/go/rpk/pkg/api/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,24 +108,6 @@ func TestSendEnvironment(t *testing.T) {
VMType: "i3.4xlarge",
OSInfo: "x86_64 5.8.9-200.fc32.x86_64 #1 SMP Mon Sep 14 18:28:45 UTC 2020 \"Fedora release 32 (Thirty Two)\"",
SentAt: time.Now(),
Config: map[string]interface{}{
"nodeUuid": "abc123-hu234-234kh",
"clusterId": "cluster 1",
"redpanda": map[string]interface{}{
"directory": "/var/lib/redpanda/",
"rpcServer": map[string]interface{}{
"address": "0.0.0.0",
"port": 33145,
},
"kafkaAPI": map[string]interface{}{
"address": "0.0.0.0",
"port": 9092,
},
},
"rpk": map[string]interface{}{
"enableUsageStats": true,
},
},
}
bs, err := json.Marshal(body)
require.NoError(t, err)
Expand Down
57 changes: 15 additions & 42 deletions src/go/rpk/pkg/cli/cmd/redpanda/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
package redpanda

import (
"encoding/json"
"errors"
"fmt"
"os"
Expand Down Expand Up @@ -178,7 +177,7 @@ func NewStartCommand(
}
seedServers, err := parseSeeds(seeds)
if err != nil {
sendEnv(fs, mgr, env, conf, !prestartCfg.checkEnabled, err)
sendEnv(fs, env, conf, !prestartCfg.checkEnabled, err)
return err
}
if len(seedServers) != 0 {
Expand All @@ -197,7 +196,7 @@ func NewStartCommand(
config.DefaultKafkaPort,
)
if err != nil {
sendEnv(fs, mgr, env, conf, !prestartCfg.checkEnabled, err)
sendEnv(fs, env, conf, !prestartCfg.checkEnabled, err)
return err
}
if len(kafkaAPI) > 0 {
Expand All @@ -216,7 +215,7 @@ func NewStartCommand(
config.DefaultProxyPort,
)
if err != nil {
sendEnv(fs, mgr, env, conf, !prestartCfg.checkEnabled, err)
sendEnv(fs, env, conf, !prestartCfg.checkEnabled, err)
return err
}
if len(proxyAPI) > 0 {
Expand All @@ -238,7 +237,7 @@ func NewStartCommand(
config.DefaultSchemaRegPort,
)
if err != nil {
sendEnv(fs, mgr, env, conf, !prestartCfg.checkEnabled, err)
sendEnv(fs, env, conf, !prestartCfg.checkEnabled, err)
return err
}
if len(schemaRegAPI) > 0 {
Expand All @@ -257,7 +256,7 @@ func NewStartCommand(
config.Default().Redpanda.RPCServer.Port,
)
if err != nil {
sendEnv(fs, mgr, env, conf, !prestartCfg.checkEnabled, err)
sendEnv(fs, env, conf, !prestartCfg.checkEnabled, err)
return err
}
if rpcServer != nil {
Expand All @@ -276,7 +275,7 @@ func NewStartCommand(
config.DefaultKafkaPort,
)
if err != nil {
sendEnv(fs, mgr, env, conf, !prestartCfg.checkEnabled, err)
sendEnv(fs, env, conf, !prestartCfg.checkEnabled, err)
return err
}
if advKafkaAPI != nil {
Expand All @@ -295,7 +294,7 @@ func NewStartCommand(
config.DefaultProxyPort,
)
if err != nil {
sendEnv(fs, mgr, env, conf, !prestartCfg.checkEnabled, err)
sendEnv(fs, env, conf, !prestartCfg.checkEnabled, err)
return err
}
if advProxyAPI != nil {
Expand All @@ -314,15 +313,15 @@ func NewStartCommand(
config.Default().Redpanda.RPCServer.Port,
)
if err != nil {
sendEnv(fs, mgr, env, conf, !prestartCfg.checkEnabled, err)
sendEnv(fs, env, conf, !prestartCfg.checkEnabled, err)
return err
}
if advRPCApi != nil {
conf.Redpanda.AdvertisedRPCAPI = advRPCApi
}
installDirectory, err := cli.GetOrFindInstallDir(fs, installDirFlag)
if err != nil {
sendEnv(fs, mgr, env, conf, !prestartCfg.checkEnabled, err)
sendEnv(fs, env, conf, !prestartCfg.checkEnabled, err)
return err
}
rpArgs, err := buildRedpandaFlags(
Expand All @@ -334,7 +333,7 @@ func NewStartCommand(
!prestartCfg.checkEnabled,
)
if err != nil {
sendEnv(fs, mgr, env, conf, !prestartCfg.checkEnabled, err)
sendEnv(fs, env, conf, !prestartCfg.checkEnabled, err)
return err
}
checkPayloads, tunerPayloads, err := prestart(
Expand All @@ -347,17 +346,17 @@ func NewStartCommand(
env.Checks = checkPayloads
env.Tuners = tunerPayloads
if err != nil {
sendEnv(fs, mgr, env, conf, !prestartCfg.checkEnabled, err)
sendEnv(fs, env, conf, !prestartCfg.checkEnabled, err)
return err
}

err = mgr.Write(conf)
if err != nil {
sendEnv(fs, mgr, env, conf, !prestartCfg.checkEnabled, err)
sendEnv(fs, env, conf, !prestartCfg.checkEnabled, err)
return err
}

sendEnv(fs, mgr, env, conf, !prestartCfg.checkEnabled, nil)
sendEnv(fs, env, conf, !prestartCfg.checkEnabled, nil)
rpArgs.ExtraArgs = args
log.Info(common.FeedbackMsg)
log.Info("Starting redpanda...")
Expand Down Expand Up @@ -928,37 +927,11 @@ func parseNamedAddress(
}, nil
}

func sendEnv(
fs afero.Fs,
mgr config.Manager,
env api.EnvironmentPayload,
conf *config.Config,
skipChecks bool,
err error,
) {
func sendEnv(fs afero.Fs, env api.EnvironmentPayload, conf *config.Config, skipChecks bool, err error) {
if err != nil {
env.ErrorMsg = err.Error()
}
// The config.Config struct holds only a subset of everything that can
// go in the YAML config file, so try to read the file directly to
// send everything.
confJSON, err := mgr.ReadAsJSON(conf.ConfigFile)
if err != nil {
log.Warnf(
"Couldn't parse latest config at '%s' due to: %s",
conf.ConfigFile,
err,
)
confBytes, err := json.Marshal(conf)
if err != nil {
log.Warnf(
"Couldn't marshal the loaded config: %s",
err,
)
}
confJSON = string(confBytes)
}
err = api.SendEnvironment(fs, env, *conf, confJSON, skipChecks)
err = api.SendEnvironment(fs, env, *conf, skipChecks)
if err != nil {
log.Debugf("couldn't send environment data: %v", err)
}
Expand Down
43 changes: 0 additions & 43 deletions src/go/rpk/pkg/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1899,49 +1899,6 @@ func TestCheckConfig(t *testing.T) {
}
}

func TestReadAsJSON(t *testing.T) {
tests := []struct {
name string
before func(fs afero.Fs) error
path string
expected string
expectedErrMsg string
}{
{
name: "it should load the config as JSON",
before: func(fs afero.Fs) error {
conf := Default()
conf.Redpanda.KafkaAPI[0].Name = "internal"
mgr := NewManager(fs)
return mgr.Write(conf)
},
path: Default().ConfigFile,
expected: `{"config_file":"/etc/redpanda/redpanda.yaml","pandaproxy":{},"redpanda":{"admin":[{"address":"0.0.0.0","port":9644}],"data_directory":"/var/lib/redpanda/data","developer_mode":true,"kafka_api":[{"address":"0.0.0.0","name":"internal","port":9092}],"node_id":0,"rpc_server":{"address":"0.0.0.0","port":33145},"seed_servers":[]},"rpk":{"coredump_dir":"/var/lib/redpanda/coredump","enable_memory_locking":false,"enable_usage_stats":false,"overprovisioned":false,"tune_aio_events":false,"tune_ballast_file":false,"tune_clocksource":false,"tune_coredump":false,"tune_cpu":false,"tune_disk_irq":false,"tune_disk_nomerges":false,"tune_disk_scheduler":false,"tune_disk_write_cache":false,"tune_fstrim":false,"tune_network":false,"tune_swappiness":false,"tune_transparent_hugepages":false},"schema_registry":{}}`,
},
{
name: "it should fail if the the config isn't found",
path: Default().ConfigFile,
expectedErrMsg: "open /etc/redpanda/redpanda.yaml: file does not exist",
},
}
for _, tt := range tests {
t.Run(tt.name, func(st *testing.T) {
fs := afero.NewMemMapFs()
mgr := NewManager(fs)
if tt.before != nil {
require.NoError(st, tt.before(fs))
}
actual, err := mgr.ReadAsJSON(tt.path)
if tt.expectedErrMsg != "" {
require.EqualError(st, err, tt.expectedErrMsg)
return
}
require.NoError(st, err)
require.Equal(st, tt.expected, actual)
})
}
}

func TestWriteAndGenerateNodeUuid(t *testing.T) {
fs := afero.NewMemMapFs()
mgr := NewManager(fs)
Expand Down
25 changes: 0 additions & 25 deletions src/go/rpk/pkg/config/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"strings"

"github.com/google/uuid"
"github.com/icza/dyno"
"github.com/mitchellh/mapstructure"
"github.com/redpanda-data/redpanda/src/go/rpk/pkg/utils"
log "github.com/sirupsen/logrus"
Expand All @@ -47,8 +46,6 @@ type Manager interface {
// Tries reading a config file at the given path, or tries to find it in
// the default locations if it doesn't exist.
ReadOrFind(path string) (*Config, error)
// Reads the configuration as JSON
ReadAsJSON(path string) (string, error)
// Generates and writes the node's UUID
WriteNodeUUID(conf *Config) error
// Merges an input config to the currently-loaded map
Expand Down Expand Up @@ -139,18 +136,6 @@ func (m *manager) ReadOrFind(path string) (*Config, error) {
return m.Read(path)
}

func (m *manager) ReadAsJSON(path string) (string, error) {
confMap, err := m.readMap(path)
if err != nil {
return "", err
}
confJSON, err := json.Marshal(confMap)
if err != nil {
return "", err
}
return string(confJSON), nil
}

func (m *manager) Read(path string) (*Config, error) {
// If the path was set, try reading only from there.
abs, err := filepath.Abs(path)
Expand All @@ -170,16 +155,6 @@ func (m *manager) Read(path string) (*Config, error) {
return conf, err
}

func (m *manager) readMap(path string) (map[string]interface{}, error) {
m.v.SetConfigFile(path)
err := m.v.ReadInConfig()
if err != nil {
return nil, err
}
strMap := dyno.ConvertMapI2MapS(m.v.AllSettings())
return strMap.(map[string]interface{}), nil
}

func (m *manager) WriteNodeUUID(conf *Config) error {
id, err := uuid.NewUUID()
if err != nil {
Expand Down

0 comments on commit 4b4ec2c

Please sign in to comment.