THREESCALE-10092 refactor status reporting on proxyConfigPromote #863

MStokluska · 2023-09-04T10:26:40Z

What
Slight refactor to proxyConfigPromote so that the status is updated even if there's an error of updating to production if there's no actual updates to be done. Jira: https://issues.redhat.com/browse/THREESCALE-10092

NOTE: Some unit tests have been slightly refactored - there's more refactors to unit tests to be done in future Jiras, for example, configuring mock client that could be used within different tests instead of relying on proxyPromote tests etc.

To Verify
From this branch run:

Create project

oc new-project threescale

Create dummy secret

kubectl apply -f - <<EOF
---
kind: Secret
apiVersion: v1
metadata: 
  name: s3-credentials
  namespace: threescale
data:
  AWS_ACCESS_KEY_ID: UkVQTEFDRV9NRQ==
  AWS_BUCKET: UkVQTEFDRV9NRQ==
  AWS_REGION: UkVQTEFDRV9NRQ==
  AWS_SECRET_ACCESS_KEY: UkVQTEFDRV9NRQ==
type: Opaque
EOF

Fetch cluster domain

DOMAIN=$(oc get routes console -n openshift-console -o json | jq -r '.status.ingress[0].routerCanonicalHostname' | sed 's/router-default.//')

Create APIM and let 3scale fully install

kubectl apply -f - <<EOF                                                        
---
apiVersion: apps.3scale.net/v1alpha1
kind: APIManager
metadata:
  name: apimanager-sample
  namespace: threescale
spec:
  system:
    fileStorage:
      simpleStorageService:
        configurationSecretRef:
          name: s3-credentials
  wildcardDomain: $DOMAIN
EOF

Create backend

kubectl apply -f - <<EOF                                                        
---
apiVersion: capabilities.3scale.net/v1beta1
kind: Backend
metadata:
  name: backend1-cr
  namespace: threescale
spec:
  mappingRules: 
    - httpMethod: GET
      increment: 1
      last: true
      metricMethodRef: hits
      pattern: /
  name: backend1
  privateBaseURL: 'https://api.example.com'
  systemName: backend1
EOF

Scenario 1
Successful reconciliation - going through stage and prod without explicit bump to stage

Create product

kubectl apply -f - <<EOF                                                        
---
apiVersion: capabilities.3scale.net/v1beta1
kind: Product
metadata:
  name: product1-cr
  namespace: threescale
spec:
  name: product1
  backendUsages:
    backend1:
      path: /
EOF

Create proxyConfigPromote

kubectl apply -f - <<EOF                                                        
---
apiVersion: capabilities.3scale.net/v1beta1
kind: ProxyConfigPromote
metadata:
  name: product1-v1-production
  namespace: threescale
spec:
  productCRName: product1-cr
  production: true
  deleteCR: true
EOF

Expected outcome:
The ProxyConfigPromote CR is deleted
The product got promoted to production
Scenario 2
Successful promotion first to stage, then in seperate CR to production:

create product CR

kubectl apply -f - <<EOF                                                        
---
apiVersion: capabilities.3scale.net/v1beta1
kind: Product
metadata:
  name: product2-cr
  namespace: threescale
spec:
  name: product2
  backendUsages:
    backend1:
      path: /
EOF

create proxy promote

apiVersion: capabilities.3scale.net/v1beta1
kind: ProxyConfigPromote
metadata:
  name: product2-v1-stage
  namespace: threescale
spec:
  productCRName: product2-cr
EOF

The outcome is:
Product is created and promoted to stage, not production, the proxyConfigPromote CR status is updated with appropriate status. The CR is not deleted

Create second proxyConfigPromote CR

apiVersion: capabilities.3scale.net/v1beta1
kind: ProxyConfigPromote
metadata:
  name: product2-v1-stage
  namespace: threescale
spec:
  productCRName: product2-cr
  production: true
EOF

The outcome is:
Product got bumped to production in 3scale, the CR is not deleted.
Now delete the proxy promote CR by applying the flag to the above resource:

deleteCR: true

The CR should be deleted.
Scenario 3
Verify that the status of proxyConfigPromote CR has been applied although no updates has been done.

create another product

kubectl apply -f - <<EOF                                                        
---
apiVersion: capabilities.3scale.net/v1beta1
kind: Product
metadata:
  name: product3-cr
  namespace: threescale
spec:
  name: product3
  backendUsages:
    backend1:
      path: /
EOF

create proxyConfigPromote to promote to production

kubectl apply -f - <<EOF                                                        
---
apiVersion: capabilities.3scale.net/v1beta1
kind: ProxyConfigPromote
metadata:
  name: product2-v1-stage
  namespace: threescale
spec:
  productCRName: product3-cr
  production: true
  deleteCR: true
EOF

Product is now promoted and the CR is gone, apply the same CR again

kubectl apply -f - <<EOF                                                        
---
apiVersion: capabilities.3scale.net/v1beta1
kind: ProxyConfigPromote
metadata:
  name: product2-v1-stage
  namespace: threescale
spec:
  productCRName: product3-cr
  production: true
  deleteCR: true
EOF

The outcome is that the proxyConfigPromote CR has updated status with an error (coming from 3scale) on why was it not successful. The Deletion should NOT be triggered as user should have decide himself on what to do next.

MStokluska · 2023-09-04T10:57:04Z

controllers/capabilities/application_status_reconciler_test.go

-	log := logf.Log.WithName("Application status reconciler test")
-	ctx := context.TODO()
-	// Objects to track in the fake client.
-	objs := []runtime.Object{getApplicationCR(), getProviderAccount(), getApiManger(), getApplicationProductList(), getApplicationDeveloperAccount(), getProviderAccountRefSecret()}


This was deleted however, I can restore with refactor of the getBaseReconciler func but wanted to make it in a seperate PR that refactors the unit tests.

hmm yea I was wondering why there were changes to the application. I will take another look in the morning.

Seems that the application tests pull the "baseReconciler" which was declared in the proxyConfigPromote.
The idea I have in mind is as part of unit test refactor to make the "baseReconciler" a util func that can be used for any of the unit tests instead. But refactor of unit tests would be done as a seperate PR since I want to figure out how to test entire reconcile func instead of the inner functions of each controller reconcile.

austincunningham · 2023-09-04T10:59:56Z

👀

MStokluska · 2023-09-04T12:27:14Z

controllers/capabilities/proxyconfigpromote_controller.go

 			_, err = threescaleAPIClient.PromoteProxyConfig(productIDStr, "sandbox", strconv.Itoa(stageElement.ProxyConfig.Version), "production")
 			if err != nil {
-				statusReconciler := NewProxyConfigPromoteStatusReconciler(r.BaseReconciler, proxyConfigPromote, "Failed", productIDStr, latestProductionVersion, latestStagingVersion, err)
+				// The version can already be in the production meaning that it can't be updated again, the proxyPromote is not going to be deleted by the operator but instead, will notify the user of the issue
+				statusReconciler := NewProxyConfigPromoteStatusReconciler(r.BaseReconciler, proxyConfigPromote, string(capabilitiesv1beta1.ProxyPromoteConfigFailedConditionType), productIDStr, latestProductionVersion, latestStagingVersion, err)


hey @austincunningham can you think of any other potentiall errors we could here apart from the comms error?
When there's nothing to promote the error returned in status isn't too informative IMO and customer might find it confusing on why it's failing. I was thinking of adding a custom error message here - something of "can't promote to production since there's no staged changes". WDYT?

Yea I was thinking the same this is not informative enough to figure out what is happening for a user .

message: >- error calling 3scale system - reason: {"version":["has already been taken"]} - code: 422

Its not an error, version is already taken doesn't inform the user to the issue. How about can't promote to stage or production as no product changes detected

I like that yes. Will update

"can't promote to production as no product changes detected, delete the proxyConfigPromote CR or introduce changes to stage env first to proceed"

austincunningham · 2023-09-05T07:50:40Z

Verification

Install the operator and create instance of apimanger
create backend

c get backends backend1-cr -oyaml | yq '.status'
backendId: 3
conditions:
  - lastTransitionTime: "2023-09-04T11:46:41Z"
    status: "False"
    type: Failed
  - lastTransitionTime: "2023-09-04T11:46:41Z"
    status: "False"
    type: Invalid
  - lastTransitionTime: "2023-09-04T11:46:41Z"
    status: "True"
    type: Synced
observedGeneration: 2
providerAccountHost: https://3scale-admin.apps.aucunnin.xcze.s1.devshift.org

Scenario 1

Create product

oc get products product1-cr -oyaml | yq '.'    
apiVersion: capabilities.3scale.net/v1beta1
kind: Product
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"capabilities.3scale.net/v1beta1","kind":"Product","metadata":{"annotations":{},"name":"product1-cr","namespace":"3scale-test"},"spec":{"backendUsages":{"backend1":{"path":"/"}},"name":"product1"}}
  creationTimestamp: "2023-09-04T11:48:46Z"
  finalizers:
    - product.capabilities.3scale.net/finalizer
  generation: 2
  name: product1-cr
  namespace: 3scale-test
  resourceVersion: "310763"
  uid: 9f68fe07-f2bb-4eb3-8ea3-e0fb11e774db
spec:
  backendUsages:
    backend1:
      path: /
  metrics:
    hits:
      description: Number of API hits
      friendlyName: Hits
      unit: hit
  name: product1
  policies:
    - configuration: {}
      configurationRef: {}
      enabled: true
      name: apicast
      version: builtin
  systemName: product1
status:
  conditions:
    - lastTransitionTime: "2023-09-04T11:48:49Z"
      status: "False"
      type: Failed
    - lastTransitionTime: "2023-09-04T11:48:49Z"
      status: "False"
      type: Invalid
    - lastTransitionTime: "2023-09-04T11:48:49Z"
      status: "False"
      type: Orphan
    - lastTransitionTime: "2023-09-04T11:48:49Z"
      status: "True"
      type: Synced
  observedGeneration: 2
  productId: 3
  providerAccountHost: https://3scale-admin.apps.aucunnin.xcze.s1.devshift.org

Promote product to production and delete the CR

Scenario 2

Create Product 2 CR

oc get products product2-cr -oyaml | yq '.'
apiVersion: capabilities.3scale.net/v1beta1
kind: Product
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"capabilities.3scale.net/v1beta1","kind":"Product","metadata":{"annotations":{},"name":"product2-cr","namespace":"3scale-test"},"spec":{"backendUsages":{"backend1":{"path":"/"}},"name":"product2"}}
  creationTimestamp: "2023-09-04T11:54:19Z"
  finalizers:
    - product.capabilities.3scale.net/finalizer
  generation: 2
  name: product2-cr
  namespace: 3scale-test
  resourceVersion: "313820"
  uid: f04ac3ea-2949-48ca-8a60-c471b05f49bf
spec:
  backendUsages:
    backend1:
      path: /
  metrics:
    hits:
      description: Number of API hits
      friendlyName: Hits
      unit: hit
  name: product2
  policies:
    - configuration: {}
      configurationRef: {}
      enabled: true
      name: apicast
      version: builtin
  systemName: product2
status:
  conditions:
    - lastTransitionTime: "2023-09-04T11:54:21Z"
      status: "False"
      type: Failed
    - lastTransitionTime: "2023-09-04T11:54:21Z"
      status: "False"
      type: Invalid
    - lastTransitionTime: "2023-09-04T11:54:21Z"
      status: "False"
      type: Orphan
    - lastTransitionTime: "2023-09-04T11:54:21Z"
      status: "True"
      type: Synced
  observedGeneration: 2
  productId: 4
  providerAccountHost: https://3scale-admin.apps.aucunnin.xcze.s1.devshift.org

promote to stage

apiVersion: capabilities.3scale.net/v1beta1
kind: ProxyConfigPromote
metadata:
  creationTimestamp: '2023-09-05T07:15:11Z'
  generation: 1
  managedFields: ...
  name: product2-v1-stage
  namespace: 3scale-test
  resourceVersion: '504422'
  uid: de291568-60d0-476e-8b04-00ce8a08de95
spec:
  productCRName: product2-cr
status:
  conditions:
    - lastTransitionTime: '2023-09-05T07:15:11Z'
      message: >-
        3scale product has been successfully promoted, any further interactions
        with this CR (apart from deletion) won't be applied
      status: 'True'
      type: Ready
  latestStagingVersion: 1
  productId: '7'

promote to production

apiVersion: capabilities.3scale.net/v1beta1
kind: ProxyConfigPromote
metadata:
  creationTimestamp: '2023-09-05T07:18:01Z'
  generation: 1
  managedFields: ...
  name: product2-v1-prod
  namespace: 3scale-test
  resourceVersion: '505944'
  uid: b6e0a7db-45cd-47e9-a67e-8dffc4afd494
spec:
  productCRName: product2-cr
  production: true
status:
  conditions:
    - lastTransitionTime: '2023-09-05T07:18:01Z'
      message: >-
        3scale product has been successfully promoted, any further interactions
        with this CR (apart from deletion) won't be applied
      status: 'True'
      type: Ready
  latestProductionVersion: 1
  latestStagingVersion: 1
  productId: '7'

set the delete flag on the production proxyConfigPromote CR and cr is deleted

only one proxyconfigpromote cr present

oc get proxyconfigpromote                  
NAME                AGE
product2-v1-stage   28m

Scenario 3

create product 3

apiVersion: capabilities.3scale.net/v1beta1
kind: Product
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: >
      {"apiVersion":"capabilities.3scale.net/v1beta1","kind":"Product","metadata":{"annotations":{},"name":"product3-cr","namespace":"3scale-test"},"spec":{"backendUsages":{"backend1":{"path":"/"}},"name":"product3"}}
  resourceVersion: '357089'
  name: product3-cr
  uid: 01509ee9-5fa3-4f8b-9182-f73f432620f9
  creationTimestamp: '2023-09-04T13:10:30Z'
  generation: 2
  managedFields: ...
  namespace: 3scale-test
  finalizers:
    - product.capabilities.3scale.net/finalizer
spec:
  backendUsages:
    backend1:
      path: /
  metrics:
    hits:
      description: Number of API hits
      friendlyName: Hits
      unit: hit
  name: product3
  policies:
    - configuration: {}
      configurationRef: {}
      enabled: true
      name: apicast
      version: builtin
  systemName: product3
status:
  conditions:
    - lastTransitionTime: '2023-09-04T13:10:33Z'
      status: 'False'
      type: Failed
    - lastTransitionTime: '2023-09-04T13:10:33Z'
      status: 'False'
      type: Invalid
    - lastTransitionTime: '2023-09-04T13:10:33Z'
      status: 'False'
      type: Orphan
    - lastTransitionTime: '2023-09-04T13:10:33Z'
      status: 'True'
      type: Synced
  observedGeneration: 2
  productId: 6
  providerAccountHost: 'https://3scale-admin.apps.aucunnin.xcze.s1.devshift.org'

promote to production
promote to production again with no changes

apiVersion: capabilities.3scale.net/v1beta1
kind: ProxyConfigPromote
metadata:
  creationTimestamp: '2023-09-05T07:48:08Z'
  generation: 1
  managedFields: ...
  name: product3-v1-prod
  namespace: 3scale-test
  resourceVersion: '522822'
  uid: 32771d32-c8b1-4456-8028-dcf4fbb43c21
spec:
  deleteCR: true
  productCRName: product3-cr
  production: true
status:
  conditions:
    - lastTransitionTime: '2023-09-05T07:48:08Z'
      message: >-
        can't promote to production as no product changes detected, delete the
        proxyConfigPromote CR or introduce changes to stage env first to proceed
      status: 'True'
      type: Failed
  latestProductionVersion: 1
  latestStagingVersion: 1
  productId: '6'

/lgtm

MStokluska · 2023-09-06T11:22:33Z

@austincunningham thanks, after reviewing the other controllers status reporting I've made an improvement to the way I was reporting status. Unfortunately this means we need another review. Sorry!

…trollers

codeclimate · 2023-09-06T17:54:56Z

Code Climate has analyzed commit b6cd305 and detected 3 issues on this pull request.

Here's the issue category breakdown:

Category	Count
Complexity	1
Duplication	1
Style	1

View more on Code Climate.

austincunningham · 2023-09-07T07:16:50Z

MStokluska requested a review from a team as a code owner September 4, 2023 10:26

openshift-merge-robot added the needs-rebase label Sep 4, 2023

MStokluska added 2 commits September 4, 2023 11:53

THREESCALE-10092 refactor status reporting on proxyConfigPromote

9f8415f

THREESCALE-10092 fix unit tests

b8b57b1

MStokluska force-pushed the THREESCALE-10092 branch from fa2a795 to b8b57b1 Compare September 4, 2023 10:54

openshift-merge-robot removed the needs-rebase label Sep 4, 2023

MStokluska commented Sep 4, 2023

View reviewed changes

austincunningham added the verified label Sep 5, 2023

THREESCALE-10092 edit error message on promoting from stage to prod

7d72e71

austincunningham added the lgtm label Sep 5, 2023

MStokluska force-pushed the THREESCALE-10092 branch from a84cef6 to 7bae410 Compare September 6, 2023 14:53

THREESCALE-10092 improve states to be in line whats done in other con…

b6cd305

…trollers

MStokluska force-pushed the THREESCALE-10092 branch from 7bae410 to b6cd305 Compare September 6, 2023 17:54

MStokluska merged commit fe6725c into 3scale:master Sep 7, 2023
14 checks passed

mkudlej mentioned this pull request Oct 9, 2023

Condition spec for proxy promotes has wrong doc #878

Closed

MStokluska mentioned this pull request Oct 16, 2023

THREESCALE-10092 fix promote to stage with false production value #881

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

THREESCALE-10092 refactor status reporting on proxyConfigPromote #863

THREESCALE-10092 refactor status reporting on proxyConfigPromote #863

MStokluska commented Sep 4, 2023 •

edited

MStokluska Sep 4, 2023

austincunningham Sep 4, 2023

MStokluska Sep 5, 2023

austincunningham commented Sep 4, 2023

MStokluska Sep 4, 2023

austincunningham Sep 4, 2023

MStokluska Sep 5, 2023

MStokluska Sep 5, 2023

austincunningham commented Sep 5, 2023

MStokluska commented Sep 6, 2023

codeclimate bot commented Sep 6, 2023

austincunningham commented Sep 7, 2023 •

edited

THREESCALE-10092 refactor status reporting on proxyConfigPromote #863

THREESCALE-10092 refactor status reporting on proxyConfigPromote #863

Conversation

MStokluska commented Sep 4, 2023 • edited

MStokluska Sep 4, 2023

Choose a reason for hiding this comment

austincunningham Sep 4, 2023

Choose a reason for hiding this comment

MStokluska Sep 5, 2023

Choose a reason for hiding this comment

austincunningham commented Sep 4, 2023

MStokluska Sep 4, 2023

Choose a reason for hiding this comment

austincunningham Sep 4, 2023

Choose a reason for hiding this comment

MStokluska Sep 5, 2023

Choose a reason for hiding this comment

MStokluska Sep 5, 2023

Choose a reason for hiding this comment

austincunningham commented Sep 5, 2023

Verification

Scenario 1

Scenario 2

Scenario 3

MStokluska commented Sep 6, 2023

codeclimate bot commented Sep 6, 2023

austincunningham commented Sep 7, 2023 • edited

Verification

Scenario 1

Scenario 2

Scenario 3

MStokluska commented Sep 4, 2023 •

edited

austincunningham commented Sep 7, 2023 •

edited