Skip to content

Commit 2c09e19

Browse files
LiilyZhangLiilyZhang
committed
Issue open-horizon#4166 - Agbot should avoid sending agreement update message if business policy is unchanged
Signed-off-by: Le Zhang <[email protected]>
1 parent ac65f84 commit 2c09e19

File tree

5 files changed

+116
-42
lines changed

5 files changed

+116
-42
lines changed

‎agreementbot/consumer_protocol_handler.go‎

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -392,18 +392,29 @@ func (b *BaseConsumerProtocolHandler) HandlePolicyChangeForAgreement(ag persiste
392392
glog.Infof(BCPHlogstring(b.Name(), fmt.Sprintf("attempting to update agreement %v due to change in policy", ag.CurrentAgreementId)))
393393
}
394394

395+
msgPrinter := i18n.GetMessagePrinter()
396+
395397
svcAllPol := externalpolicy.ExternalPolicy{}
398+
svcPolicyHandler := exchange.GetHTTPServicePolicyHandler(b)
399+
svcResolveHandler := exchange.GetHTTPServiceDefResolverHandler(b)
396400

397401
for _, svcId := range ag.ServiceId {
398-
if svcPol, err := exchange.GetServicePolicyWithId(b, svcId); err != nil {
399-
glog.Errorf(BCPHlogstring(b.Name(), fmt.Sprintf("failed to get service policy for %v from the exchange: %v", svcId, err)))
402+
if svcDef, err := exchange.GetServiceWithId(b, svcId); err != nil {
403+
glog.Errorf(BCPHlogstring(b.Name(), fmt.Sprintf("failed to get service %v, error: %v", svcId, err)))
400404
return false, false, false
401-
} else if svcPol != nil {
402-
svcAllPol.MergeWith(&svcPol.ExternalPolicy, false)
405+
} else if svcDef != nil {
406+
if mergedSvcPol, _, _, _, _, err := compcheck.GetServicePolicyWithDefaultProperties(svcPolicyHandler, svcResolveHandler, svcDef.URL, exchange.GetOrg(svcId), svcDef.Version, svcDef.Arch, msgPrinter); err != nil {
407+
glog.Errorf(BCPHlogstring(b.Name(), fmt.Sprintf("failed to get merged service policy for %v, error: %v", svcId, err)))
408+
return false, false, false
409+
} else if mergedSvcPol != nil {
410+
svcAllPol.MergeWith(mergedSvcPol, false)
411+
}
403412
}
404413
}
405414

406-
msgPrinter := i18n.GetMessagePrinter()
415+
if glog.V(5) {
416+
glog.Infof(BCPHlogstring(b.Name(), fmt.Sprintf("For agreement %v merged svc policy is %v", ag.CurrentAgreementId, svcAllPol)))
417+
}
407418

408419
busPolHandler := exchange.GetHTTPBusinessPoliciesHandler(b)
409420
_, busPol, err := compcheck.GetBusinessPolicy(busPolHandler, ag.PolicyName, true, msgPrinter)
@@ -510,7 +521,7 @@ func (b *BaseConsumerProtocolHandler) HandlePolicyChangeForAgreement(ag persiste
510521
}
511522
return true, true, false
512523
}
513-
// new cluster namespace is still compatible
524+
// cluster namespace remains same
514525
}
515526
}
516527

@@ -535,6 +546,13 @@ func (b *BaseConsumerProtocolHandler) HandlePolicyChangeForAgreement(ag persiste
535546
}
536547
}
537548

549+
if same, msg := consumerPol.IsSamePolicy(oldPolicy); same {
550+
glog.V(3).Infof("business policy(producerPol) %v content remains same with old policy", ag.PolicyName)
551+
return true, true, true
552+
} else {
553+
glog.V(3).Infof("business policy %v content changed: %v", ag.PolicyName, msg)
554+
}
555+
538556
newTsCs, err := policy.Create_Terms_And_Conditions(producerPol, consumerPol, wl, ag.CurrentAgreementId, b.config.AgreementBot.DefaultWorkloadPW, b.config.AgreementBot.NoDataIntervalS, basicprotocol.PROTOCOL_CURRENT_VERSION)
539557
if err != nil {
540558
glog.Errorf(BCPHlogstring(b.Name(), fmt.Sprintf("error creating new terms and conditions: %v", err)))
@@ -543,6 +561,7 @@ func (b *BaseConsumerProtocolHandler) HandlePolicyChangeForAgreement(ag persiste
543561

544562
ag.LastPolicyUpdateTime = uint64(time.Now().Unix())
545563

564+
// this function will send out "basicagreementupdate"
546565
b.UpdateAgreement(&ag, basicprotocol.MsgUpdateTypePolicyChange, newTsCs, cph)
547566

548567
return true, true, true

‎compcheck/comp_check.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1132,7 +1132,7 @@ func GetServiceAndDeps(svcUrl, svcOrg, svcVersion, svcArch string,
11321132
// not found, get it and dependents from the exchange
11331133
_, depSvcs, exchTopSvc, topId, err = getServiceResolvedDef(svcUrl, svcOrg, svcVersion, svcArch)
11341134
if err != nil {
1135-
return nil, "", nil, NewCompCheckError(fmt.Errorf(msgPrinter.Sprintf("Failed to find definition for dependent services of %s. Compatability of %s cannot be fully evaluated until all services are in the Exchange.", topId, externalpolicy.PROP_NODE_PRIVILEGED)), COMPCHECK_EXCHANGE_ERROR)
1135+
return nil, "", nil, NewCompCheckError(fmt.Errorf(msgPrinter.Sprintf("Failed to find definition for dependent services of %s (%s/%s/%s/%s), error: %v. Compatability of %s cannot be fully evaluated until all services are in the Exchange.", topId, svcUrl, svcOrg, svcVersion, svcArch, err, externalpolicy.PROP_NODE_PRIVILEGED)), COMPCHECK_EXCHANGE_ERROR)
11361136
}
11371137
topSvc = &ServiceDefinition{exchange.GetOrg(topId), *exchTopSvc}
11381138
}

‎exchange/service.go‎

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -687,11 +687,15 @@ func ServiceDefResolver(wURL string, wOrg string, wVersion string, wArch string,
687687
// Make sure the required service has the same arch as the service.
688688
// Convert version to a version range expression (if it's not already an expression) so that the underlying GetService
689689
// will return us something in the range required by the service.
690+
serviceVersion := sDep.Version
691+
if serviceVersion == "" {
692+
serviceVersion = sDep.VersionRange
693+
}
690694
var serviceDef *ServiceDefinition
691695
if sDep.Arch != wArch {
692696
return nil, nil, nil, "", errors.New(fmt.Sprintf("service %v has a different architecture than the top level service.", sDep))
693-
} else if vExp, err := semanticversion.Version_Expression_Factory(sDep.Version); err != nil {
694-
return nil, nil, nil, "", errors.New(fmt.Sprintf("unable to create version expression from %v, error %v", sDep.Version, err))
697+
} else if vExp, err := semanticversion.Version_Expression_Factory(serviceVersion); err != nil {
698+
return nil, nil, nil, "", errors.New(fmt.Sprintf("unable to create version expression from version or version range %v, error %v", serviceVersion, err))
695699
} else if apiSpecs, s_map, s_def, s_id, err := ServiceDefResolver(sDep.URL, sDep.Org, vExp.Get_expression(), sDep.Arch, serviceHandler); err != nil {
696700
return nil, nil, nil, "", err
697701
} else {
@@ -722,6 +726,52 @@ func ServiceDefResolver(wURL string, wOrg string, wVersion string, wArch string,
722726
}
723727
}
724728

729+
// Retrieve the service object from the exchange. The service_id is prefixed with the org name.
730+
// It returns nil if there is no such service with given service_id. Service_id is in format: <org>/<Id>
731+
func GetServiceWithId(ec ExchangeContext, service_id string) (*ServiceDefinition, error) {
732+
glog.V(3).Infof(rpclogString(fmt.Sprintf("getting service policy for %v.", service_id)))
733+
734+
// Get the service object. There should only be 1.
735+
var resp interface{}
736+
resp = new(GetServicesResponse)
737+
var svc ServiceDefinition
738+
739+
targetURL := fmt.Sprintf("%vorgs/%v/services/%v", ec.GetExchangeURL(), GetOrg(service_id), GetId(service_id))
740+
741+
retryCount := ec.GetHTTPFactory().RetryCount
742+
retryInterval := ec.GetHTTPFactory().GetRetryInterval()
743+
for {
744+
if err, tpErr := InvokeExchange(ec.GetHTTPFactory().NewHTTPClient(nil), "GET", targetURL, ec.GetExchangeId(), ec.GetExchangeToken(), nil, &resp); err != nil {
745+
glog.Errorf(rpclogString(fmt.Sprintf(err.Error())))
746+
return nil, err
747+
} else if tpErr != nil {
748+
glog.Warningf(rpclogString(fmt.Sprintf(tpErr.Error())))
749+
if ec.GetHTTPFactory().RetryCount == 0 {
750+
time.Sleep(time.Duration(retryInterval) * time.Second)
751+
continue
752+
} else if retryCount == 0 {
753+
return nil, fmt.Errorf("Exceeded %v retries for error: %v", ec.GetHTTPFactory().RetryCount, tpErr)
754+
} else {
755+
retryCount--
756+
time.Sleep(time.Duration(retryInterval) * time.Second)
757+
continue
758+
}
759+
} else {
760+
glog.V(3).Infof(rpclogString(fmt.Sprintf("returning service for %v.", service_id)))
761+
services := resp.(*GetServicesResponse)
762+
if len(services.Services) == 1 {
763+
var cachedSvcDefs map[string]ServiceDefinition
764+
svc = services.Services[service_id]
765+
updateServiceDefCache(services.Services, cachedSvcDefs, GetOrg(service_id), svc.URL, svc.Arch)
766+
} else {
767+
glog.V(3).Infof(rpclogString(fmt.Sprintf("service %v not found.", service_id)))
768+
return nil, nil
769+
}
770+
return &svc, nil
771+
}
772+
}
773+
}
774+
725775
// This function gets the image docker auths for a service.
726776
func GetServiceDockerAuths(ec ExchangeContext, url string, org string, version string, arch string) ([]ImageDockerAuth, error) {
727777

‎policy/policy_file.go‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,41 @@ func (self *Policy) ShortString() string {
535535
return res
536536
}
537537

538+
func (self *Policy) IsSamePolicy(compare *Policy) (bool, string) {
539+
misMatchString := ""
540+
isSame := false
541+
if compare == nil {
542+
misMatchString = fmt.Sprintf("Nil policy to comapre with policy %v", self.Header)
543+
} else if !self.Header.IsSame(compare.Header) {
544+
misMatchString = fmt.Sprintf("Header %v mismatch with %v", self.Header, compare.Header)
545+
} else if (len(compare.Workloads) == 0 || (len(compare.Workloads) != 0 && compare.Workloads[0].WorkloadURL == "")) && !self.APISpecs.IsSame(compare.APISpecs, true) {
546+
misMatchString = fmt.Sprintf("API Spec %v mismatch with %v", self.APISpecs, compare.APISpecs)
547+
} else if !self.AgreementProtocols.IsSame(compare.AgreementProtocols) {
548+
misMatchString = fmt.Sprintf("AgreementProtocol %v mismatch with %v", self.AgreementProtocols, compare.AgreementProtocols)
549+
} else if !self.IsSameWorkload(compare) {
550+
misMatchString = fmt.Sprintf("Workload %v mismatch with %v", self.Workloads, compare.Workloads)
551+
} else if !self.DataVerify.IsSame(compare.DataVerify) {
552+
misMatchString = fmt.Sprintf("DataVerify %v mismatch with %v", self.DataVerify, compare.DataVerify)
553+
} else if !self.Properties.IsSame(compare.Properties) {
554+
misMatchString = fmt.Sprintf("Properties %v mismatch with %v", self.Properties, compare.Properties)
555+
} else if !self.Constraints.IsSame(compare.Constraints) {
556+
misMatchString = fmt.Sprintf("Constraints %v mismatch with %v", self.Constraints, compare.Constraints)
557+
} else if self.RequiredWorkload != compare.RequiredWorkload {
558+
misMatchString = fmt.Sprintf("RequiredWorkload %v mismatch with %v", self.RequiredWorkload, compare.RequiredWorkload)
559+
} else if self.MaxAgreements != compare.MaxAgreements {
560+
misMatchString = fmt.Sprintf("MaxAgreement %v mismatch with %v", self.MaxAgreements, compare.MaxAgreements)
561+
} else if !UserInputArrayIsSame(self.UserInput, compare.UserInput) {
562+
misMatchString = fmt.Sprintf("UserInput %v mismatch with %v", self.UserInput, compare.UserInput)
563+
} else if !exchangecommon.SecretBindingIsSame(self.SecretBinding, compare.SecretBinding) {
564+
misMatchString = fmt.Sprintf("SecretBinding %v mismatch with %v", self.SecretBinding, compare.SecretBinding)
565+
} else {
566+
isSame = true
567+
}
568+
569+
return isSame, misMatchString
570+
571+
}
572+
538573
func (self *Policy) IsSameWorkload(compare *Policy) bool {
539574
if len(self.Workloads) != len(compare.Workloads) {
540575
return false

‎policy/policy_manager.go‎

Lines changed: 3 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"github.com/golang/glog"
88
"github.com/open-horizon/anax/config"
99
"github.com/open-horizon/anax/cutil"
10-
"github.com/open-horizon/anax/exchangecommon"
1110
"sync"
1211
)
1312

@@ -267,42 +266,13 @@ func (self *PolicyManager) hasPolicy(org string, matchPolicy *Policy) (bool, err
267266
return false, errors.New(fmt.Sprintf("organization %v not found", org))
268267
}
269268

269+
var isSame bool
270270
for _, pol := range orgArray {
271271
if errString != "" {
272272
glog.V(5).Infof("Policy Manager: Previous search loop returned: %v", errString)
273273
}
274-
if !pol.Header.IsSame(matchPolicy.Header) {
275-
errString = fmt.Sprintf("Header %v mismatch with %v", pol.Header, matchPolicy.Header)
276-
continue
277-
} else if (len(matchPolicy.Workloads) == 0 || (len(matchPolicy.Workloads) != 0 && matchPolicy.Workloads[0].WorkloadURL == "")) && !pol.APISpecs.IsSame(matchPolicy.APISpecs, true) {
278-
errString = fmt.Sprintf("API Spec %v mismatch with %v", pol.APISpecs, matchPolicy.APISpecs)
279-
continue
280-
} else if !pol.AgreementProtocols.IsSame(matchPolicy.AgreementProtocols) {
281-
errString = fmt.Sprintf("AgreementProtocol %v mismatch with %v", pol.AgreementProtocols, matchPolicy.AgreementProtocols)
282-
continue
283-
} else if !pol.IsSameWorkload(matchPolicy) {
284-
errString = fmt.Sprintf("Workload %v mismatch with %v", pol.Workloads, matchPolicy.Workloads)
285-
continue
286-
} else if !pol.DataVerify.IsSame(matchPolicy.DataVerify) {
287-
errString = fmt.Sprintf("DataVerify %v mismatch with %v", pol.DataVerify, matchPolicy.DataVerify)
288-
continue
289-
} else if !pol.Properties.IsSame(matchPolicy.Properties) {
290-
errString = fmt.Sprintf("Properties %v mismatch with %v", pol.Properties, matchPolicy.Properties)
291-
continue
292-
} else if !pol.Constraints.IsSame(matchPolicy.Constraints) {
293-
errString = fmt.Sprintf("Constraints %v mismatch with %v", pol.Constraints, matchPolicy.Constraints)
294-
continue
295-
} else if pol.RequiredWorkload != matchPolicy.RequiredWorkload {
296-
errString = fmt.Sprintf("RequiredWorkload %v mismatch with %v", pol.RequiredWorkload, matchPolicy.RequiredWorkload)
297-
continue
298-
} else if pol.MaxAgreements != matchPolicy.MaxAgreements {
299-
errString = fmt.Sprintf("MaxAgreement %v mismatch with %v", pol.MaxAgreements, matchPolicy.MaxAgreements)
300-
continue
301-
} else if !UserInputArrayIsSame(pol.UserInput, matchPolicy.UserInput) {
302-
errString = fmt.Sprintf("UserInput %v mismatch with %v", pol.UserInput, matchPolicy.UserInput)
303-
continue
304-
} else if !exchangecommon.SecretBindingIsSame(pol.SecretBinding, matchPolicy.SecretBinding) {
305-
errString = fmt.Sprintf("SecretBinding %v mismatch with %v", pol.SecretBinding, matchPolicy.SecretBinding)
274+
275+
if isSame, errString = pol.IsSamePolicy(matchPolicy); !isSame {
306276
continue
307277
} else {
308278
errString = ""

0 commit comments

Comments
 (0)