Automatic merge from release/2.3.0 -> release/2.4.0

* commit 'ff4b6ecad93cd9c6c6d83e4fe31019c51c951d41':
  cloudcommon: validators: return 400 for struct validation failure
  loadbalancers: show loadbalancer name for listener and backend group
  mcclient: loadbalancers: allow updating names
  loadbalancers: more compact backendGroup validation
  lbagent: list with details=true explicitly
  lbagent: fix unused imports
  pkg: httputils: ParseJSONResponse(): rewrite for clearness
  mcclient: base: guard against empty repsonse
  mcclient: options: set details correctly

cmd/climc/shell/loadbalancerbackendgroups.go

@@ -39,6 +39,18 @@ func init() {
 		printList(result, modules.LoadbalancerBackendGroups.GetColumns(s))
 		return nil
 	})
+	R(&options.LoadbalancerBackendGroupUpdateOptions{}, "lbbackendgroup-update", "Update lbbackendgroup", func(s *mcclient.ClientSession, opts *options.LoadbalancerBackendGroupUpdateOptions) error {
+		params, err := options.StructToParams(opts)
+		if err != nil {
+			return err
+		}
+		lbbackendgroup, err := modules.LoadbalancerBackendGroups.Update(s, opts.ID, params)
+		if err != nil {
+			return err
+		}
+		printObject(lbbackendgroup)
+		return nil
+	})
 	R(&options.LoadbalancerBackendGroupDeleteOptions{}, "lbbackendgroup-delete", "Show lbbackendgroup", func(s *mcclient.ClientSession, opts *options.LoadbalancerBackendGroupDeleteOptions) error {
 		lbbackendgroup, err := modules.LoadbalancerBackendGroups.Delete(s, opts.ID, nil)
 		if err != nil {

pkg/cloudcommon/validators/validators.go

@@ -511,7 +511,7 @@ func (v *ValidatorStruct) Validate(data *jsonutils.JSONDict) error {
 	if valueValidator, ok := v.Value.(IValidatorBase); ok {
 		err = valueValidator.Validate(data)
 		if err != nil {
-			return err
+			return newInvalidValueError(v.Key, err.Error())
 		}
 	}
 	data.Set(v.Key, jsonutils.Marshal(v.Value))

pkg/compute/models/loadbalanceracls.go

@@ -101,9 +101,12 @@ type SLoadbalancerAcl struct {
 	AclEntries *SLoadbalancerAclEntries `list:"user" update:"user" create:"required"`
 }
 
-func loadbalancerAclsValidateAclEntries(data *jsonutils.JSONDict) (*jsonutils.JSONDict, error) {
+func loadbalancerAclsValidateAclEntries(data *jsonutils.JSONDict, update bool) (*jsonutils.JSONDict, error) {
 	aclEntries := SLoadbalancerAclEntries{}
 	aclEntriesV := validators.NewStructValidator("acl_entries", &aclEntries)
+	if update {
+		aclEntriesV.Optional(true)
+	}
 	err := aclEntriesV.Validate(data)
 	if err != nil {
 		return nil, err
@@ -112,7 +115,7 @@ func loadbalancerAclsValidateAclEntries(data *jsonutils.JSONDict) (*jsonutils.JS
 }
 
 func (man *SLoadbalancerAclManager) ValidateCreateData(ctx context.Context, userCred mcclient.TokenCredential, ownerProjId string, query jsonutils.JSONObject, data *jsonutils.JSONDict) (*jsonutils.JSONDict, error) {
-	data, err := loadbalancerAclsValidateAclEntries(data)
+	data, err := loadbalancerAclsValidateAclEntries(data, false)
 	if err != nil {
 		return nil, err
 	}
@@ -124,7 +127,7 @@ func (lbacl *SLoadbalancerAcl) AllowPerformStatus(ctx context.Context, userCred
 }
 
 func (lbacl *SLoadbalancerAcl) ValidateUpdateData(ctx context.Context, userCred mcclient.TokenCredential, query jsonutils.JSONObject, data *jsonutils.JSONDict) (*jsonutils.JSONDict, error) {
-	data, err := loadbalancerAclsValidateAclEntries(data)
+	data, err := loadbalancerAclsValidateAclEntries(data, true)
 	if err != nil {
 		return nil, err
 	}

pkg/compute/models/loadbalancerbackendgroups.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 
 	"yunion.io/x/jsonutils"
+	"yunion.io/x/log"
 	"yunion.io/x/sqlchemy"
 
 	"yunion.io/x/onecloud/pkg/cloudcommon/db"
@@ -102,6 +103,25 @@ func (lbbg *SLoadbalancerBackendGroup) ValidateDeleteCondition(ctx context.Conte
 	return nil
 }
 
+func (lbbg *SLoadbalancerBackendGroup) GetCustomizeColumns(ctx context.Context, userCred mcclient.TokenCredential, query jsonutils.JSONObject) *jsonutils.JSONDict {
+	extra := lbbg.SVirtualResourceBase.GetCustomizeColumns(ctx, userCred, query)
+	{
+		lb, err := LoadbalancerManager.FetchById(lbbg.LoadbalancerId)
+		if err != nil {
+			log.Errorf("loadbalancer backend group %s(%s): fetch loadbalancer (%s) error: %s",
+				lbbg.Name, lbbg.Id, lbbg.LoadbalancerId, err)
+			return extra
+		}
+		extra.Set("loadbalancer", jsonutils.NewString(lb.GetName()))
+	}
+	return extra
+}
+
+func (lbbg *SLoadbalancerBackendGroup) GetExtraDetails(ctx context.Context, userCred mcclient.TokenCredential, query jsonutils.JSONObject) *jsonutils.JSONDict {
+	extra := lbbg.GetCustomizeColumns(ctx, userCred, query)
+	return extra
+}
+
 func (lbbg *SLoadbalancerBackendGroup) PreDelete(ctx context.Context, userCred mcclient.TokenCredential) {
 	lbbg.DoPendingDelete(ctx, userCred)
 	lbbg.PreDeleteSubs(ctx, userCred)

pkg/compute/models/loadbalancerlistenerrules.go

@@ -112,8 +112,7 @@ func (man *SLoadbalancerListenerRuleManager) ValidateCreateData(ctx context.Cont
 		return nil, fmt.Errorf("listener type must be http/https, got %s", listenerType)
 	}
 	{
-		backendGroup := backendGroupV.Model.(*SLoadbalancerBackendGroup)
-		if backendGroup.LoadbalancerId != listener.LoadbalancerId {
+		if backendGroup, ok := backendGroupV.Model.(*SLoadbalancerBackendGroup); ok && backendGroup.LoadbalancerId != listener.LoadbalancerId {
 			return nil, httperrors.NewInputParameterError("backend group %s(%s) belongs to loadbalancer %s instead of %s",
 				backendGroup.Name, backendGroup.Id, backendGroup.LoadbalancerId, listener.LoadbalancerId)
 		}
@@ -136,14 +135,13 @@ func (lbr *SLoadbalancerListenerRule) ValidateUpdateData(ctx context.Context, us
 	if err != nil {
 		return nil, err
 	}
-	if backendGroupV.Model != nil {
+	if backendGroup, ok := backendGroupV.Model.(*SLoadbalancerBackendGroup); ok && backendGroup.Id != lbr.BackendGroupId {
 		listenerM, err := LoadbalancerListenerManager.FetchById(lbr.ListenerId)
 		if err != nil {
 			return nil, httperrors.NewInputParameterError("loadbalancerlistenerrule %s(%s): fetching listener %s failed",
 				lbr.Name, lbr.Id, lbr.ListenerId)
 		}
 		listener := listenerM.(*SLoadbalancerListener)
-		backendGroup := backendGroupV.Model.(*SLoadbalancerBackendGroup)
 		if backendGroup.LoadbalancerId != listener.LoadbalancerId {
 			return nil, httperrors.NewInputParameterError("backend group %s(%s) belongs to loadbalancer %s instead of %s",
 				backendGroup.Name, backendGroup.Id, backendGroup.LoadbalancerId, listener.LoadbalancerId)

pkg/compute/models/loadbalancerlisteners.go

@@ -217,12 +217,9 @@ func (man *SLoadbalancerListenerManager) ValidateCreateData(ctx context.Context,
 		}
 	}
 	{
-		if backendGroupV.Model != nil {
-			backendGroup := backendGroupV.Model.(*SLoadbalancerBackendGroup)
-			if backendGroup.LoadbalancerId != lb.Id {
-				return nil, httperrors.NewInputParameterError("backend group %s(%s) belongs to loadbalancer %s instead of %s",
-					backendGroup.Name, backendGroup.Id, backendGroup.LoadbalancerId, lb.Id)
-			}
+		if backendGroup, ok := backendGroupV.Model.(*SLoadbalancerBackendGroup); ok && backendGroup.LoadbalancerId != lb.Id {
+			return nil, httperrors.NewInputParameterError("backend group %s(%s) belongs to loadbalancer %s instead of %s",
+				backendGroup.Name, backendGroup.Id, backendGroup.LoadbalancerId, lb.Id)
 		}
 	}
 	{
@@ -348,12 +345,9 @@ func (lblis *SLoadbalancerListener) ValidateUpdateData(ctx context.Context, user
 		}
 	}
 	{
-		if backendGroupV.Model != nil {
-			backendGroup := backendGroupV.Model.(*SLoadbalancerBackendGroup)
-			if backendGroup.LoadbalancerId != lblis.LoadbalancerId {
-				return nil, httperrors.NewInputParameterError("backend group %s(%s) belongs to loadbalancer %s instead of %s",
-					backendGroup.Name, backendGroup.Id, backendGroup.LoadbalancerId, lblis.LoadbalancerId)
-			}
+		if backendGroup, ok := backendGroupV.Model.(*SLoadbalancerBackendGroup); ok && backendGroup.LoadbalancerId != lblis.LoadbalancerId {
+			return nil, httperrors.NewInputParameterError("backend group %s(%s) belongs to loadbalancer %s instead of %s",
+				backendGroup.Name, backendGroup.Id, backendGroup.LoadbalancerId, lblis.LoadbalancerId)
 		}
 	}
 	return lblis.SVirtualResourceBase.ValidateUpdateData(ctx, userCred, query, data)
@@ -361,16 +355,27 @@ func (lblis *SLoadbalancerListener) ValidateUpdateData(ctx context.Context, user
 
 func (lblis *SLoadbalancerListener) GetCustomizeColumns(ctx context.Context, userCred mcclient.TokenCredential, query jsonutils.JSONObject) *jsonutils.JSONDict {
 	extra := lblis.SVirtualResourceBase.GetCustomizeColumns(ctx, userCred, query)
-	if lblis.BackendGroupId == "" {
-		return extra
+	{
+		lb, err := LoadbalancerManager.FetchById(lblis.LoadbalancerId)
+		if err != nil {
+			log.Errorf("loadbalancer listener %s(%s): fetch loadbalancer (%s) error: %s",
+				lblis.Name, lblis.Id, lblis.LoadbalancerId, err)
+			return extra
+		}
+		extra.Set("loadbalancer", jsonutils.NewString(lb.GetName()))
 	}
-	lbbg, err := LoadbalancerBackendGroupManager.FetchById(lblis.BackendGroupId)
-	if err != nil {
-		log.Errorf("loadbalancer listener %s(%s): fetch backend group (%s) error: %s",
-			lblis.Name, lblis.Id, lblis.BackendGroupId, err)
-		return extra
+	{
+		if lblis.BackendGroupId == "" {
+			return extra
+		}
+		lbbg, err := LoadbalancerBackendGroupManager.FetchById(lblis.BackendGroupId)
+		if err != nil {
+			log.Errorf("loadbalancer listener %s(%s): fetch backend group (%s) error: %s",
+				lblis.Name, lblis.Id, lblis.BackendGroupId, err)
+			return extra
+		}
+		extra.Set("backend_group", jsonutils.NewString(lbbg.GetName()))
 	}
-	extra.Set("backend_group", jsonutils.NewString(lbbg.GetName()))
 	return extra
 }
 

pkg/compute/models/loadbalancers.go

@@ -169,7 +169,7 @@ func (lb *SLoadbalancer) ValidateUpdateData(ctx context.Context, userCred mcclie
 	if err != nil {
 		return nil, err
 	}
-	if backendGroup := backendGroupV.Model.(*SLoadbalancerBackendGroup); backendGroup.LoadbalancerId != lb.Id {
+	if backendGroup, ok := backendGroupV.Model.(*SLoadbalancerBackendGroup); ok && backendGroup.LoadbalancerId != lb.Id {
 		return nil, httperrors.NewInputParameterError("backend group %s(%s) belongs to loadbalancer %s, not %s",
 			backendGroup.Name, backendGroup.Id, backendGroup.LoadbalancerId, lb.Id)
 	}

pkg/lbagent/models/keepalived.go

@@ -2,13 +2,10 @@ package models
 
 import (
 	"bytes"
-	"fmt"
 	"io/ioutil"
 	"path/filepath"
 	"text/template"
 
-	"yunion.io/x/log"
-
 	agentutils "yunion.io/x/onecloud/pkg/lbagent/utils"
 )
 

pkg/lbagent/models/reflect.go

@@ -87,6 +87,7 @@ func GetModels(opts *GetModelsOptions) error {
 
 	listOptions := options.BaseListOptions{
 		Admin:   options.Bool(true),
+		Details: options.Bool(true),
 		Filter:  []string{minUpdatedAtFilter(minUpdatedAt)},
 		OrderBy: []string{"updated_at", "id"},
 		Order:   "asc",

pkg/mcclient/modules/base.go

@@ -113,6 +113,9 @@ func (this *BaseManager) _list(session *mcclient.ClientSession, path, responseKe
 	if err != nil {
 		return nil, err
 	}
+	if body == nil {
+		return nil, fmt.Errorf("empty response")
+	}
 	rets, err := body.GetArray(responseKey)
 	if err != nil {
 		return nil, err

pkg/mcclient/options/base.go

@@ -197,8 +197,5 @@ func (opts *BaseListOptions) Params() (*jsonutils.JSONDict, error) {
 			params.Set("admin", jsonutils.JSONTrue)
 		}
 	}
-	if opts.Details == nil {
-		params.Set("details", jsonutils.JSONFalse)
-	}
 	return params, nil
 }

pkg/mcclient/options/base_test.go

@@ -201,3 +201,24 @@ func TestOptionsStructToParams(t *testing.T) {
 		testSs(t, cases)
 	})
 }
+
+func TestBaseListOptions(t *testing.T) {
+	t.Run("pending-delete-all", func(t *testing.T) {
+		opts := &BaseListOptions{
+			PendingDeleteAll: Bool(true),
+		}
+		params, err := opts.Params()
+		if err != nil {
+			t.Fatalf("unexpected error: %s", err)
+		}
+		for _, f := range []string{"details", "admin"} {
+			got, err := params.Bool(f)
+			if err != nil {
+				t.Fatalf("getting %s field failed: %s", f, err)
+			}
+			if !got {
+				t.Errorf("expecting %s=true, got false", f)
+			}
+		}
+	})
+}

pkg/mcclient/options/loadbalanceracls.go

@@ -69,7 +69,9 @@ type LoadbalancerAclListOptions struct {
 }
 
 type LoadbalancerAclUpdateOptions struct {
-	ID       string   `json:"-"`
+	ID   string `json:"-"`
+	Name string
+
 	AclEntry []string `help:"acl entry with cidr and comment separated by #, e.g. 10.9.0.0/16#no comment" json:"-"`
 }
 
@@ -99,9 +101,13 @@ func (opts *LoadbalancerAclUpdateOptions) Params() (*jsonutils.JSONDict, error)
 	if err != nil {
 		return nil, err
 	}
-	aclEntries := NewAclEntries(opts.AclEntry)
-	aclEntriesJson := jsonutils.Marshal(aclEntries)
-	params.Set("acl_entries", aclEntriesJson)
+	// - when it's nil, we leave it alone without updating
+	// - when it's non-nil, we update it as a whole
+	if opts.AclEntry != nil {
+		aclEntries := NewAclEntries(opts.AclEntry)
+		aclEntriesJson := jsonutils.Marshal(aclEntries)
+		params.Set("acl_entries", aclEntriesJson)
+	}
 	return params, nil
 }
 

pkg/mcclient/options/loadbalanceragents.go

@@ -86,7 +86,8 @@ type LoadbalancerAgentGetOptions struct {
 }
 
 type LoadbalancerAgentUpdateOptions struct {
-	ID string
+	ID   string
+	Name string
 
 	HbTimeout *int
 

pkg/mcclient/options/loadbalancerbackendgroups.go

@@ -4,12 +4,20 @@ type LoadbalancerBackendGroupCreateOptions struct {
 	NAME         string
 	Loadbalancer string
 }
+
 type LoadbalancerBackendGroupGetOptions struct {
 	ID string
 }
+
+type LoadbalancerBackendGroupUpdateOptions struct {
+	ID   string
+	Name string
+}
+
 type LoadbalancerBackendGroupDeleteOptions struct {
 	ID string
 }
+
 type LoadbalancerBackendGroupListOptions struct {
 	BaseListOptions
 	Loadbalancer string

pkg/mcclient/options/loadbalancerbackends.go

@@ -19,7 +19,8 @@ type LoadbalancerBackendListOptions struct {
 }
 
 type LoadbalancerBackendUpdateOptions struct {
-	ID string
+	ID   string
+	Name string
 
 	Weight *int
 	Port   *int

pkg/mcclient/options/loadbalancercertificates.go

@@ -70,7 +70,8 @@ type LoadbalancerCertificateListOptions struct {
 }
 
 type LoadbalancerCertificateUpdateOptions struct {
-	ID string
+	ID   string
+	Name string
 
 	Cert string `json:"-" help:"path to certificate file"`
 	Pkey string `json:"-" help:"path to private key file"`

pkg/mcclient/options/loadbalancerlistenerrules.go

@@ -18,7 +18,9 @@ type LoadbalancerListenerRuleListOptions struct {
 }
 
 type LoadbalancerListenerRuleUpdateOptions struct {
-	ID           string
+	ID   string
+	Name string
+
 	BackendGroup string
 }
 

pkg/mcclient/options/loadbalancerlisteners.go

@@ -97,7 +97,8 @@ type LoadbalancerListenerListOptions struct {
 }
 
 type LoadbalancerListenerUpdateOptions struct {
-	ID string
+	ID   string
+	Name string
 
 	BackendGroup string
 

pkg/mcclient/options/loadbalancers.go

@@ -11,7 +11,9 @@ type LoadbalancerGetOptions struct {
 }
 
 type LoadbalancerUpdateOptions struct {
-	ID           string
+	ID   string
+	Name string
+
 	BackendGroup string
 }
 

pkg/util/httputils/httputils.go

@@ -8,6 +8,7 @@ import (
 	"io/ioutil"
 	"net/http"
 	"net/url"
+	"os"
 	"strconv"
 	"strings"
 
@@ -15,7 +16,6 @@ import (
 	"github.com/moul/http2curl"
 
 	"yunion.io/x/jsonutils"
-	"yunion.io/x/log"
 	"yunion.io/x/pkg/gotypes"
 	"yunion.io/x/pkg/trace"
 
@@ -189,62 +189,58 @@ func ParseJSONResponse(resp *http.Response, err error, debug bool) (http.Header,
 	}
 	rbody, err := ioutil.ReadAll(resp.Body)
 	if debug {
-		fmt.Println(string(rbody))
+		fmt.Fprintf(os.Stderr, "%s\n", string(rbody))
 	}
 	if err != nil {
 		return nil, nil, fmt.Errorf("Fail to read body: %s", err)
 	}
+
 	var jrbody jsonutils.JSONObject = nil
 	if len(rbody) > 0 {
+		var err error
 		jrbody, err = jsonutils.Parse(rbody)
-
 		if err != nil && debug {
-			log.Errorf("parse JSON body %s fail: %s", rbody, err)
-		}
-		///// XXX: ignore error case
-		// if err != nil && resp.StatusCode < 300 {
-		//     return nil, nil, fmt.Errorf("Fail to decode body: %s", err)
-		// }
-		if jrbody != nil && debug {
-			fmt.Println(jrbody)
+			fmt.Fprintf(os.Stderr, "parsing json failed: %s", err)
 		}
 	}
-	if resp.StatusCode >= 300 && resp.StatusCode < 400 {
+
+	if resp.StatusCode < 300 {
+		return resp.Header, jrbody, nil
+	} else if resp.StatusCode >= 300 && resp.StatusCode < 400 {
 		ce := JSONClientError{}
 		ce.Code = resp.StatusCode
 		ce.Details = resp.Header.Get("Location")
 		ce.Class = "redirect"
 		return nil, nil, &ce
-	} else if resp.StatusCode >= 400 {
+	} else {
 		ce := JSONClientError{}
+
 		if jrbody == nil {
 			ce.Code = resp.StatusCode
 			ce.Details = resp.Status
 			return nil, nil, &ce
-		} else {
-			jrbody2, e := jrbody.Get("error")
-			if e == nil {
-				ecode, e := jrbody2.Int("code")
-				if e == nil {
-					ce.Code = int(ecode)
-					ce.Details, _ = jrbody2.GetString("message")
-					ce.Class, _ = jrbody2.GetString("title")
-					return nil, nil, &ce
-				} else {
-					ce.Code = resp.StatusCode
-					ce.Details = jrbody2.String()
-					return nil, nil, &ce
-				}
+		}
+
+		jrbody2, err := jrbody.Get("error")
+		if err == nil {
+			ecode, err := jrbody2.Int("code")
+			if err == nil {
+				ce.Code = int(ecode)
+				ce.Details, _ = jrbody2.GetString("message")
+				ce.Class, _ = jrbody2.GetString("title")
+				return nil, nil, &ce
 			} else {
-				err = jrbody.Unmarshal(&ce)
-				if err != nil {
-					return nil, nil, err
-				} else {
-					return nil, nil, &ce
-				}
+				ce.Code = resp.StatusCode
+				ce.Details = jrbody2.String()
+				return nil, nil, &ce
 			}
 		}
-	} else {
-		return resp.Header, jrbody, nil
+
+		err = jrbody.Unmarshal(&ce)
+		if err != nil {
+			return nil, nil, err
+		} else {
+			return nil, nil, &ce
+		}
 	}
 }