Skip to content

Commit

Permalink
Address comments & update mocks
Browse files Browse the repository at this point in the history
  • Loading branch information
@gemmahou
gemmahou committed Aug 28, 2024
1 parent 20b8fe4 commit bf6bc4e
Show file tree
Hide file tree
Showing 5 changed files with 147 additions and 43 deletions.
8 changes: 4 additions & 4 deletions mockgcp/mockcompute/regionalforwardingrulev1.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func (s *RegionalForwardingRulesV1) Insert(ctx context.Context, req *pb.InsertFo
OperationType: PtrTo("insert"),
User: PtrTo("[email protected]"),
}
return s.startGlobalLRO(ctx, name.Project.ID, op, func() (proto.Message, error) {
return s.startRegionalLRO(ctx, name.Project.ID, name.Region, op, func() (proto.Message, error) {
return obj, nil
})
}
Expand All @@ -134,7 +134,7 @@ func (s *RegionalForwardingRulesV1) Delete(ctx context.Context, req *pb.DeleteFo
OperationType: PtrTo("delete"),
User: PtrTo("[email protected]"),
}
return s.startGlobalLRO(ctx, name.Project.ID, op, func() (proto.Message, error) {
return s.startRegionalLRO(ctx, name.Project.ID, name.Region, op, func() (proto.Message, error) {
return deleted, nil
})
}
Expand Down Expand Up @@ -166,7 +166,7 @@ func (s *RegionalForwardingRulesV1) SetLabels(ctx context.Context, req *pb.SetLa
// SetLabels operation has EndTime in response
EndTime: PtrTo("2024-04-01T12:34:56.123456Z"),
}
return s.startGlobalLRO(ctx, name.Project.ID, op, func() (proto.Message, error) {
return s.startRegionalLRO(ctx, name.Project.ID, name.Region, op, func() (proto.Message, error) {
return obj, nil
})
}
Expand Down Expand Up @@ -196,7 +196,7 @@ func (s *RegionalForwardingRulesV1) SetTarget(ctx context.Context, req *pb.SetTa
OperationType: PtrTo("SetTarget"),
User: PtrTo("[email protected]"),
}
return s.startGlobalLRO(ctx, name.Project.ID, op, func() (proto.Message, error) {
return s.startRegionalLRO(ctx, name.Project.ID, name.Region, op, func() (proto.Message, error) {
return obj, nil
})
}
Expand Down
1 change: 0 additions & 1 deletion pkg/controller/direct/compute/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ type optionsRoundTripper struct {
func (m *optionsRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
if m.config.UserAgent != "" {
req.Header.Set("User-Agent", m.config.UserAgent)
req.Header.Del("x-goog-request-params")
}
return m.inner.RoundTrip(req)
}
Expand Down
37 changes: 16 additions & 21 deletions pkg/controller/direct/compute/forwardingrule_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ func (a *forwardingRuleAdapter) Find(ctx context.Context) (bool, error) {
if direct.IsNotFound(err) {
return false, nil
}
return false, fmt.Errorf("getting ComputeForwardingRule %q failed: %w", a.fullyQualifiedName(), err)
return false, fmt.Errorf("getting ComputeForwardingRule %q: %w", a.fullyQualifiedName(), err)
}
} else {
req := &computepb.GetForwardingRuleRequest{
Expand All @@ -221,7 +221,7 @@ func (a *forwardingRuleAdapter) Find(ctx context.Context) (bool, error) {
if direct.IsNotFound(err) {
return false, nil
}
return false, fmt.Errorf("getting ComputeForwardingRule %q failed: %w", a.fullyQualifiedName(), err)
return false, fmt.Errorf("getting ComputeForwardingRule %q: %w", a.fullyQualifiedName(), err)
}
}
a.actual = forwardingRule
Expand Down Expand Up @@ -266,7 +266,11 @@ func (a *forwardingRuleAdapter) Create(ctx context.Context, u *unstructured.Unst
op, err = a.forwardingRulesClient.Insert(ctx, req)
}
if err != nil {
return fmt.Errorf("creating ComputeForwardingRule %s failed: %w", a.fullyQualifiedName(), err)
return fmt.Errorf("creating ComputeForwardingRule %s: %w", a.fullyQualifiedName(), err)
}
err = op.Wait(ctx)
if err != nil {
return fmt.Errorf("waiting ComputeForwardingRule %s create failed: %w", a.fullyQualifiedName(), err)
}
log.V(2).Info("successfully created ComputeForwardingRule", "name", a.fullyQualifiedName())
// Get the created resource
Expand All @@ -286,12 +290,12 @@ func (a *forwardingRuleAdapter) Create(ctx context.Context, u *unstructured.Unst
created, err = a.forwardingRulesClient.Get(ctx, getReq)
}
if err != nil {
return fmt.Errorf("getting ComputeForwardingRule %q failed: %w", a.fullyQualifiedName(), err)
return fmt.Errorf("getting ComputeForwardingRule %q: %w", a.fullyQualifiedName(), err)
}

status := &krm.ComputeForwardingRuleStatus{
LabelFingerprint: created.LabelFingerprint,
CreationTimestamp: op.Proto().InsertTime,
CreationTimestamp: created.CreationTimestamp,
SelfLink: created.SelfLink,
}
status.ExternalRef = a.id.AsExternalRef()
Expand Down Expand Up @@ -339,7 +343,7 @@ func (a *forwardingRuleAdapter) Update(ctx context.Context, u *unstructured.Unst
op, err = a.forwardingRulesClient.SetLabels(ctx, setLabelsReq)
}
if err != nil {
return fmt.Errorf("updating ComputeForwardingRule labels %s failed: %w", a.fullyQualifiedName(), err)
return fmt.Errorf("updating ComputeForwardingRule labels %s: %w", a.fullyQualifiedName(), err)
}
err = op.Wait(ctx)
if err != nil {
Expand All @@ -362,7 +366,7 @@ func (a *forwardingRuleAdapter) Update(ctx context.Context, u *unstructured.Unst
updated, err = a.forwardingRulesClient.Get(ctx, getReq)
}
if err != nil {
return fmt.Errorf("getting ComputeForwardingRule %q failed: %w", a.id.forwardingRule, err)
return fmt.Errorf("getting ComputeForwardingRule %q: %w", a.id.forwardingRule, err)
}

// setTarget request is sent when there are updates to target.
Expand All @@ -384,15 +388,14 @@ func (a *forwardingRuleAdapter) Update(ctx context.Context, u *unstructured.Unst
op, err = a.forwardingRulesClient.SetTarget(ctx, setTargetReq)
}
if err != nil {
return fmt.Errorf("updating ComputeForwardingRule target %s failed: %w", a.fullyQualifiedName(), err)
return fmt.Errorf("updating ComputeForwardingRule target %s: %w", a.fullyQualifiedName(), err)
}
err = op.Wait(ctx)
if err != nil {
return fmt.Errorf("waiting ComputeForwardingRule %s update target failed: %w", a.fullyQualifiedName(), err)
}
log.V(2).Info("successfully updated ComputeForwardingRule target", "name", a.fullyQualifiedName())
}

// Get the updated resource
if a.id.location == "global" {
getReq := &computepb.GetGlobalForwardingRuleRequest{
Expand All @@ -409,12 +412,12 @@ func (a *forwardingRuleAdapter) Update(ctx context.Context, u *unstructured.Unst
updated, err = a.forwardingRulesClient.Get(ctx, getReq)
}
if err != nil {
return fmt.Errorf("getting ComputeForwardingRule %q failed: %w", a.id.forwardingRule, err)
return fmt.Errorf("getting ComputeForwardingRule %q: %w", a.id.forwardingRule, err)
}

status := &krm.ComputeForwardingRuleStatus{
LabelFingerprint: updated.LabelFingerprint,
CreationTimestamp: op.Proto().InsertTime,
CreationTimestamp: updated.CreationTimestamp,
SelfLink: updated.SelfLink,
}
return setStatus(u, status)
Expand All @@ -434,15 +437,7 @@ func (a *forwardingRuleAdapter) Delete(ctx context.Context) (bool, error) {
log := klog.FromContext(ctx).WithName(ctrlName)
log.V(2).Info("deleting ComputeForwardingRule", "name", a.id.forwardingRule)

exist, err := a.Find(ctx)
if err != nil {
return false, err
}
if !exist {
// return (false, nil) if the object was not found but should be presumed deleted.
return false, nil
}

var err error
op := &gcp.Operation{}
if a.id.location == "global" {
req := &computepb.DeleteGlobalForwardingRuleRequest{
Expand All @@ -459,7 +454,7 @@ func (a *forwardingRuleAdapter) Delete(ctx context.Context) (bool, error) {
op, err = a.forwardingRulesClient.Delete(ctx, req)
}
if err != nil {
return false, fmt.Errorf("deleting ComputeForwardingRule %s failed: %w", a.id.forwardingRule, err)
return false, fmt.Errorf("deleting ComputeForwardingRule %s: %w", a.id.forwardingRule, err)
}
err = op.Wait(ctx)
if err != nil {
Expand Down
34 changes: 34 additions & 0 deletions ...estdata/basic/compute/v1beta1/computeforwardingrule/globalcomputeforwardingrule/_http.log
View file
Original file line number Diff line number Diff line change
Expand Up @@ -789,6 +789,40 @@ X-Xss-Protection: 0

---

GET https://compute.googleapis.com/compute/v1/projects/${projectId}/global/operations/${operationID}
Content-Type: application/json
User-Agent: kcc/controller-manager
x-goog-request-params: project=${projectId}&operation=${operationID}

200 OK
Cache-Control: private
Content-Type: application/json; charset=UTF-8
Server: ESF
Vary: Origin
Vary: X-Origin
Vary: Referer
X-Content-Type-Options: nosniff
X-Frame-Options: SAMEORIGIN
X-Xss-Protection: 0

{
"endTime": "2024-04-01T12:34:56.123456Z",
"id": "000000000000000000000",
"insertTime": "2024-04-01T12:34:56.123456Z",
"kind": "compute#operation",
"name": "${operationID}",
"operationType": "insert",
"progress": 100,
"selfLink": "https://www.googleapis.com/compute/v1/projects/${projectId}/global/operations/${operationID}",
"startTime": "2024-04-01T12:34:56.123456Z",
"status": "DONE",
"targetId": "${forwardingRulesId}",
"targetLink": "https://www.googleapis.com/compute/v1/projects/${projectId}/global/forwardingRules/computeglobalforwardingrule-${uniqueId}",
"user": "[email protected]"
}

---

GET https://compute.googleapis.com/compute/v1/projects/${projectId}/global/forwardingRules/${forwardingRuleID}
Content-Type: application/json
User-Agent: kcc/controller-manager
Expand Down
110 changes: 93 additions & 17 deletions ...tdata/basic/compute/v1beta1/computeforwardingrule/regionalcomputeforwardingrule/_http.log
View file
Original file line number Diff line number Diff line change
Expand Up @@ -551,7 +551,8 @@ X-Xss-Protection: 0
"name": "${operationID}",
"operationType": "insert",
"progress": 0,
"selfLink": "https://www.googleapis.com/compute/v1/projects/${projectId}/global/operations/${operationID}",
"region": "https://www.googleapis.com/compute/v1/projects/${projectId}/regions/us-central1",
"selfLink": "https://www.googleapis.com/compute/v1/projects/${projectId}/regions/us-central1/operations/${operationID}",
"startTime": "2024-04-01T12:34:56.123456Z",
"status": "RUNNING",
"targetId": "${forwardingRulesId}",
Expand All @@ -561,6 +562,41 @@ X-Xss-Protection: 0

---

GET https://compute.googleapis.com/compute/v1/projects/${projectId}/regions/us-central1/operations/${operationID}
Content-Type: application/json
User-Agent: kcc/controller-manager
x-goog-request-params: project=${projectId}&region=us-central1&operation=${operationID}

200 OK
Cache-Control: private
Content-Type: application/json; charset=UTF-8
Server: ESF
Vary: Origin
Vary: X-Origin
Vary: Referer
X-Content-Type-Options: nosniff
X-Frame-Options: SAMEORIGIN
X-Xss-Protection: 0

{
"endTime": "2024-04-01T12:34:56.123456Z",
"id": "000000000000000000000",
"insertTime": "2024-04-01T12:34:56.123456Z",
"kind": "compute#operation",
"name": "${operationID}",
"operationType": "insert",
"progress": 100,
"region": "https://www.googleapis.com/compute/v1/projects/${projectId}/regions/us-central1",
"selfLink": "https://www.googleapis.com/compute/v1/projects/${projectId}/regions/us-central1/operations/${operationID}",
"startTime": "2024-04-01T12:34:56.123456Z",
"status": "DONE",
"targetId": "${forwardingRulesId}",
"targetLink": "https://www.googleapis.com/compute/v1/projects/${projectId}/regions/us-central1/forwardingRules/computeregionalforwardingrule-${uniqueId}",
"user": "[email protected]"
}

---

GET https://compute.googleapis.com/compute/v1/projects/${projectId}/regions/us-central1/forwardingRules/${forwardingRuleID}
Content-Type: application/json
User-Agent: kcc/controller-manager
Expand Down Expand Up @@ -633,7 +669,8 @@ X-Xss-Protection: 0
"name": "${operationID}",
"operationType": "SetLabels",
"progress": 0,
"selfLink": "https://www.googleapis.com/compute/v1/projects/${projectId}/global/operations/${operationID}",
"region": "https://www.googleapis.com/compute/v1/projects/${projectId}/regions/us-central1",
"selfLink": "https://www.googleapis.com/compute/v1/projects/${projectId}/regions/us-central1/operations/${operationID}",
"startTime": "2024-04-01T12:34:56.123456Z",
"status": "RUNNING",
"targetId": "${forwardingRulesId}",
Expand All @@ -643,6 +680,41 @@ X-Xss-Protection: 0

---

GET https://compute.googleapis.com/compute/v1/projects/${projectId}/regions/us-central1/operations/${operationID}
Content-Type: application/json
User-Agent: kcc/controller-manager
x-goog-request-params: project=${projectId}&region=us-central1&operation=${operationID}

200 OK
Cache-Control: private
Content-Type: application/json; charset=UTF-8
Server: ESF
Vary: Origin
Vary: X-Origin
Vary: Referer
X-Content-Type-Options: nosniff
X-Frame-Options: SAMEORIGIN
X-Xss-Protection: 0

{
"endTime": "2024-04-01T12:34:56.123456Z",
"id": "000000000000000000000",
"insertTime": "2024-04-01T12:34:56.123456Z",
"kind": "compute#operation",
"name": "${operationID}",
"operationType": "SetLabels",
"progress": 100,
"region": "https://www.googleapis.com/compute/v1/projects/${projectId}/regions/us-central1",
"selfLink": "https://www.googleapis.com/compute/v1/projects/${projectId}/regions/us-central1/operations/${operationID}",
"startTime": "2024-04-01T12:34:56.123456Z",
"status": "DONE",
"targetId": "${forwardingRulesId}",
"targetLink": "https://www.googleapis.com/compute/v1/projects/${projectId}/regions/us-central1/forwardingRules/computeregionalforwardingrule-${uniqueId}",
"user": "[email protected]"
}

---

GET https://compute.googleapis.com/compute/v1/projects/${projectId}/regions/us-central1/forwardingRules/${forwardingRuleID}
Content-Type: application/json
User-Agent: kcc/controller-manager
Expand Down Expand Up @@ -705,7 +777,8 @@ X-Xss-Protection: 0
"name": "${operationID}",
"operationType": "delete",
"progress": 0,
"selfLink": "https://www.googleapis.com/compute/v1/projects/${projectId}/global/operations/${operationID}",
"region": "https://www.googleapis.com/compute/v1/projects/${projectId}/regions/us-central1",
"selfLink": "https://www.googleapis.com/compute/v1/projects/${projectId}/regions/us-central1/operations/${operationID}",
"startTime": "2024-04-01T12:34:56.123456Z",
"status": "RUNNING",
"targetId": "${forwardingRulesId}",
Expand All @@ -715,12 +788,12 @@ X-Xss-Protection: 0

---

GET https://compute.googleapis.com/compute/v1/projects/${projectId}/regions/us-central1/forwardingRules/${forwardingRuleID}
GET https://compute.googleapis.com/compute/v1/projects/${projectId}/regions/us-central1/operations/${operationID}
Content-Type: application/json
User-Agent: kcc/controller-manager
x-goog-request-params: project=${projectId}&region=us-central1&forwarding_rule=computeregionalforwardingrule-${uniqueId}
x-goog-request-params: project=${projectId}&region=us-central1&operation=${operationID}

404 Not Found
200 OK
Cache-Control: private
Content-Type: application/json; charset=UTF-8
Server: ESF
Expand All @@ -732,17 +805,20 @@ X-Frame-Options: SAMEORIGIN
X-Xss-Protection: 0

{
"error": {
"code": 404,
"errors": [
{
"domain": "global",
"message": "forwardingRule \"projects/${projectId}/regions/us-central1/forwardingRules/computeregionalforwardingrule-${uniqueId}\" not found",
"reason": "notFound"
}
],
"message": "forwardingRule \"projects/${projectId}/regions/us-central1/forwardingRules/computeregionalforwardingrule-${uniqueId}\" not found"
}
"endTime": "2024-04-01T12:34:56.123456Z",
"id": "000000000000000000000",
"insertTime": "2024-04-01T12:34:56.123456Z",
"kind": "compute#operation",
"name": "${operationID}",
"operationType": "delete",
"progress": 100,
"region": "https://www.googleapis.com/compute/v1/projects/${projectId}/regions/us-central1",
"selfLink": "https://www.googleapis.com/compute/v1/projects/${projectId}/regions/us-central1/operations/${operationID}",
"startTime": "2024-04-01T12:34:56.123456Z",
"status": "DONE",
"targetId": "${forwardingRulesId}",
"targetLink": "https://www.googleapis.com/compute/v1/projects/${projectId}/regions/us-central1/forwardingRules/computeregionalforwardingrule-${uniqueId}",
"user": "[email protected]"
}

---
Expand Down

0 comments on commit bf6bc4e

Please sign in to comment.