Support namespace ExtendedOptions in cluster spec (#282)

> LGTM, but it would be good to test on a live operated cluster before we merge, both with and without extended options. I've seen some... "unintuitive" (to put it nicely) behavior from Kubernetes before when serializing / deserializing anything non-standard.
Sure, we've done some testing on a real cluster.

Author: linasm

.golangci.yml

@@ -211,6 +211,8 @@ linters:
     # New line required before return would require a large fraction of the
     # code base to need updating, it's not worth the perceived benefit.
     - nlreturn
+    # Opinionated and sometimes wrong.
+    - paralleltest
   disable-all: false
   presets:
     # bodyclose, errcheck, gosec, govet, scopelint, staticcheck, typecheck

pkg/apis/m3dboperator/v1alpha1/namespace.go

@@ -20,6 +20,8 @@
 
 package v1alpha1
 
+import "encoding/json"
+
 // Namespace defines an M3DB namespace or points to a preset M3DB namespace.
 type Namespace struct {
 	// Name is the namespace name.
@@ -98,6 +100,12 @@ type DownsampleOptions struct {
 	All bool `json:"all,omitempty"`
 }
 
+// ExtendedOptions stores the extended namespace options.
+type ExtendedOptions struct {
+	Type    string                     `json:"type,omitempty"`
+	Options map[string]json.RawMessage `json:"options,omitempty"`
+}
+
 // NamespaceOptions defines parameters for an M3DB namespace. See
 // https://m3db.github.io/m3/operational_guide/namespace_configuration/ for more
 // details.
@@ -131,4 +139,7 @@ type NamespaceOptions struct {
 
 	// AggregationOptions sets the aggregation parameters.
 	AggregationOptions AggregationOptions `json:"aggregationOptions,omitempty"`
+
+	// ExtendedOptions stores the extended namespace options.
+	ExtendedOptions *ExtendedOptions `json:"extendedOptions,omitempty"`
 }

pkg/apis/m3dboperator/v1alpha1/zz_generated.deepcopy.go

@@ -21,12 +21,16 @@
 package namespace
 
 import (
+	"bytes"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"time"
 
 	myspec "github.com/m3db/m3db-operator/pkg/apis/m3dboperator/v1alpha1"
 
+	"github.com/gogo/protobuf/jsonpb"
+	pbtypes "github.com/gogo/protobuf/types"
 	m3ns "github.com/m3db/m3/src/dbnode/generated/proto/namespace"
 	"github.com/m3db/m3/src/query/generated/proto/admin"
 )
@@ -95,6 +99,11 @@ func m3dbNamespaceOptsFromSpec(opts *myspec.NamespaceOptions) (*m3ns.NamespaceOp
 		return nil, err
 	}
 
+	extOpts, err := m3dbExtendedOptsFromSpec(opts.ExtendedOptions)
+	if err != nil {
+		return nil, err
+	}
+
 	return &m3ns.NamespaceOptions{
 		BootstrapEnabled:   opts.BootstrapEnabled,
 		FlushEnabled:       opts.FlushEnabled,
@@ -106,6 +115,7 @@ func m3dbNamespaceOptsFromSpec(opts *myspec.NamespaceOptions) (*m3ns.NamespaceOp
 		IndexOptions:       indexOpts,
 		ColdWritesEnabled:  opts.ColdWritesEnabled,
 		AggregationOptions: aggOpts,
+		ExtendedOptions:    extOpts,
 	}, nil
 }
 
@@ -192,3 +202,33 @@ func m3dbAggregationOptsFromSpec(opts myspec.AggregationOptions) (*m3ns.Aggregat
 	}, nil
 
 }
+
+func m3dbExtendedOptsFromSpec(opts *myspec.ExtendedOptions) (*m3ns.ExtendedOptions, error) {
+	if opts == nil {
+		return nil, nil
+	}
+
+	pbStruct, err := pbStructFromSpec(opts.Options)
+	if err != nil {
+		return nil, err
+	}
+
+	return &m3ns.ExtendedOptions{
+		Type:    opts.Type,
+		Options: pbStruct,
+	}, nil
+}
+
+func pbStructFromSpec(opts map[string]json.RawMessage) (*pbtypes.Struct, error) {
+	jsonBytes, err := json.Marshal(opts)
+	if err != nil {
+		return nil, err
+	}
+
+	pbStruct := &pbtypes.Struct{}
+	if err := jsonpb.Unmarshal(bytes.NewReader(jsonBytes), pbStruct); err != nil {
+		return nil, err
+	}
+
+	return pbStruct, nil
+}

pkg/m3admin/namespace/namespace.go

@@ -21,14 +21,15 @@
 package namespace
 
 import (
+	"encoding/json"
 	"testing"
 	"time"
 
 	myspec "github.com/m3db/m3db-operator/pkg/apis/m3dboperator/v1alpha1"
 
+	pbtypes "github.com/gogo/protobuf/types"
 	m3ns "github.com/m3db/m3/src/dbnode/generated/proto/namespace"
 	"github.com/m3db/m3/src/query/generated/proto/admin"
-
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -40,29 +41,25 @@ func TestRequestFromSpec(t *testing.T) {
 	require.NoError(t, err)
 
 	tests := []struct {
+		name   string
 		ns     myspec.Namespace
 		req    *admin.NamespaceAddRequest
 		expErr bool
 	}{
 		{
+			name:   "no fields",
 			ns:     myspec.Namespace{},
 			expErr: true,
 		},
 		{
+			name: "only name set",
 			ns: myspec.Namespace{
 				Name: "empty",
 			},
 			expErr: true,
 		},
 		{
-			ns: myspec.Namespace{
-				Name:    "badpreset",
-				Preset:  "a",
-				Options: &myspec.NamespaceOptions{},
-			},
-			expErr: true,
-		},
-		{
+			name: "valid custom",
 			ns: myspec.Namespace{
 				Name: "validcustom",
 				Options: &myspec.NamespaceOptions{
@@ -111,6 +108,7 @@ func TestRequestFromSpec(t *testing.T) {
 			},
 		},
 		{
+			name: "AggregatedOptions",
 			ns: myspec.Namespace{
 				Name: "aggregated",
 				Options: &myspec.NamespaceOptions{
@@ -170,6 +168,72 @@ func TestRequestFromSpec(t *testing.T) {
 			},
 		},
 		{
+			name: "ExtendedOptions",
+			ns: myspec.Namespace{
+				Name: "extended",
+				Options: &myspec.NamespaceOptions{
+					BootstrapEnabled:  true,
+					WritesToCommitLog: false,
+					RetentionOptions: myspec.RetentionOptions{
+						RetentionPeriod:                     "1s",
+						BlockSize:                           "1s",
+						BufferFuture:                        "1s",
+						BufferPast:                          "1s",
+						BlockDataExpiry:                     true,
+						BlockDataExpiryAfterNotAccessPeriod: "1s",
+					},
+					IndexOptions: myspec.IndexOptions{
+						BlockSize: "1s",
+						Enabled:   true,
+					},
+					ExtendedOptions: &myspec.ExtendedOptions{
+						Type: "testOpts",
+						Options: map[string]json.RawMessage{
+							"key1": json.RawMessage(`"str"`),
+							"key2": json.RawMessage(`123`),
+							"key3": json.RawMessage(`{
+								"subKey1": "foo",
+								"subKey2": "bar"
+							}`),
+						},
+					},
+				},
+			},
+			req: &admin.NamespaceAddRequest{
+				Name: "extended",
+				Options: &m3ns.NamespaceOptions{
+					BootstrapEnabled:  true,
+					WritesToCommitLog: false,
+					RetentionOptions: &m3ns.RetentionOptions{
+						RetentionPeriodNanos:                     1000000000,
+						BlockSizeNanos:                           1000000000,
+						BufferFutureNanos:                        1000000000,
+						BufferPastNanos:                          1000000000,
+						BlockDataExpiry:                          true,
+						BlockDataExpiryAfterNotAccessPeriodNanos: 1000000000,
+					},
+					IndexOptions: &m3ns.IndexOptions{
+						BlockSizeNanos: 1000000000,
+						Enabled:        true,
+					},
+					ExtendedOptions: &m3ns.ExtendedOptions{
+						Type: "testOpts",
+						Options: &pbtypes.Struct{Fields: map[string]*pbtypes.Value{
+							"key1": {Kind: &pbtypes.Value_StringValue{StringValue: "str"}},
+							"key2": {Kind: &pbtypes.Value_NumberValue{NumberValue: 123}},
+							"key3": {Kind: &pbtypes.Value_StructValue{StructValue: &pbtypes.Struct{
+								Fields: map[string]*pbtypes.Value{
+									"subKey1": {Kind: &pbtypes.Value_StringValue{StringValue: "foo"}},
+									"subKey2": {Kind: &pbtypes.Value_StringValue{StringValue: "bar"}},
+								},
+							}}},
+						}},
+					},
+				},
+			},
+		},
+		{
+			name: "invalid custom",
 			ns: myspec.Namespace{
 				Name: "invalidcustom",
 				Options: &myspec.NamespaceOptions{
@@ -197,13 +261,24 @@ func TestRequestFromSpec(t *testing.T) {
 			expErr: true,
 		},
 		{
+			name: "bad preset 1",
+			ns: myspec.Namespace{
+				Name:    "badpreset",
+				Preset:  "a",
+				Options: &myspec.NamespaceOptions{},
+			},
+			expErr: true,
+		},
+		{
+			name: "bad preset 2",
 			ns: myspec.Namespace{
 				Name:   "foo",
 				Preset: "a",
 			},
 			expErr: true,
 		},
 		{
+			name: "preset 10s:2d",
 			ns: myspec.Namespace{
 				Name:   "foo",
 				Preset: "10s:2d",
@@ -214,6 +289,7 @@ func TestRequestFromSpec(t *testing.T) {
 			},
 		},
 		{
+			name: "preset 1m:40d",
 			ns: myspec.Namespace{
 				Name:   "foo",
 				Preset: "1m:40d",
@@ -226,24 +302,26 @@ func TestRequestFromSpec(t *testing.T) {
 	}
 
 	for _, test := range tests {
-		req, err := RequestFromSpec(test.ns)
-		if test.expErr {
-			assert.Error(t, err)
-		} else {
-			assert.NoError(t, err)
-			assert.Equal(t, test.req, req)
-		}
+		t.Run(test.name, func(t *testing.T) {
+			req, err := RequestFromSpec(test.ns)
+			if test.expErr {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+				assert.Equal(t, test.req, req)
+			}
+		})
 	}
 }
 
 func TestRetentionOptsFromAPI(t *testing.T) {
 	opts := myspec.RetentionOptions{
-		RetentionPeriod:                     time.Duration(time.Second).String(),
-		BlockSize:                           time.Duration(2 * time.Second).String(),
-		BufferFuture:                        time.Duration(3 * time.Second).String(),
-		BufferPast:                          time.Duration(4 * time.Second).String(),
+		RetentionPeriod:                     time.Second.String(),
+		BlockSize:                           (2 * time.Second).String(),
+		BufferFuture:                        (3 * time.Second).String(),
+		BufferPast:                          (4 * time.Second).String(),
 		BlockDataExpiry:                     true,
-		BlockDataExpiryAfterNotAccessPeriod: time.Duration(5 * time.Second).String(),
+		BlockDataExpiryAfterNotAccessPeriod: (5 * time.Second).String(),
 	}
 
 	nsOpts, err := m3dbRetentionOptsFromSpec(opts)