Skip to content

Commit

Permalink
Silent error message if clowdenvironment does not exist, pre-push, re…
Browse files Browse the repository at this point in the history
…queue true (#186)

* return error if clowdenv exists but has error

* Updating log messages

* pre push

* updated errors

* removing mutex locks

* Fixed linting issue

* Fixing err issue

* return nil on status update

* requeue on failure, return nil

* fix return value

* return err instead of nil

* revert this back to main

* update logic for retrieving reservation object

* return nil if not found

* fix log output

* Reverting file change
  • Loading branch information
maknop authored Oct 9, 2023
1 parent e7c3af6 commit 4dae07b
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 47 deletions.
18 changes: 9 additions & 9 deletions controllers/cloud.redhat.com/clowdenvironment_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/RedHatInsights/rhc-osdk-utils/utils"
"github.com/go-logr/logr"
"github.com/prometheus/client_golang/prometheus"
k8serr "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
ctrl "sigs.k8s.io/controller-runtime"
Expand All @@ -50,8 +51,12 @@ func (r *ClowdenvironmentReconciler) Reconcile(ctx context.Context, req ctrl.Req

env := clowder.ClowdEnvironment{}
if err := r.client.Get(ctx, req.NamespacedName, &env); err != nil {
log.Error(err, "Error retrieving clowdenv", "env-name", env.Name)
return ctrl.Result{}, err
if !k8serr.IsNotFound(err) {
return ctrl.Result{}, nil
}

r.log.Error(err, "there was an issue retrieving the clowdenvironment [%s]", env.Name)
return ctrl.Result{Requeue: true}, err
}

if ready, err := helpers.VerifyClowdEnvReady(env); !ready {
Expand Down Expand Up @@ -96,13 +101,8 @@ func (r *ClowdenvironmentReconciler) Reconcile(ctx context.Context, req ctrl.Req
return ctrl.Result{Requeue: true}, fmt.Errorf("could not retrieve updated namespace [%s] after updating annotations: %w", namespaceName, err)
}

namespace, err = helpers.GetNamespace(ctx, r.client, namespaceName)
if err != nil {
log.Error(err, "could not retrieve newly created namespace", "namespace", namespaceName)
}

if err := r.client.Update(ctx, &namespace); err != nil {
return ctrl.Result{}, err
if err = r.client.Update(ctx, &namespace); err != nil {
return ctrl.Result{Requeue: true}, err
}

elapsed := nsCompletionTime.Sub(namespace.CreationTimestamp.Time)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,9 @@ func (r *NamespaceReservationReconciler) Reconcile(ctx context.Context, req ctrl
// Must have been deleted
return ctrl.Result{}, nil
}
log.Error(err, "Reservation Not Found")
return ctrl.Result{}, err

r.log.Error(err, fmt.Sprintf("there was an issue retrieving the reservation object for namespace [%s]", req.NamespacedName.Namespace))
return ctrl.Result{Requeue: true}, err
}

if res.Status.Pool == "" {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
core "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
k8serr "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/types"
)

Expand Down Expand Up @@ -72,7 +72,7 @@ var _ = Describe("Reservation controller basic reservation", func() {

Eventually(func() bool {
err := k8sClient.Get(ctx, types.NamespacedName{Name: resName}, updatedReservation)
return errors.IsNotFound(err)
return k8serr.IsNotFound(err)
}, timeout, interval).Should(BeTrue())
})

Expand Down Expand Up @@ -103,7 +103,7 @@ var _ = Describe("Reservation controller basic reservation", func() {
err1 := k8sClient.Get(ctx, types.NamespacedName{Name: resName1}, updatedR1)
err2 := k8sClient.Get(ctx, types.NamespacedName{Name: resName2}, updatedR2)
err3 := k8sClient.Get(ctx, types.NamespacedName{Name: resName3}, updatedR3)
if errors.IsNotFound(err1) || err2 != nil || err3 != nil {
if k8serr.IsNotFound(err1) || err2 != nil || err3 != nil {
return false
}

Expand Down
129 changes: 96 additions & 33 deletions deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ objects:
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.8.0
controller-gen.kubebuilder.io/version: v0.11.1
creationTimestamp: null
name: namespacepools.cloud.redhat.com
spec:
Expand Down Expand Up @@ -83,6 +83,13 @@ objects:
description: Defines the Configuration for the Clowder Database
Provider.
properties:
caBundleURL:
description: Indicates where Clowder will fetch the
database CA certificate bundle from. Currently only
used in (*_app-interface_*) mode. If none is specified,
the AWS RDS combined CA bundle is used.
pattern: ^https?:\/\/.+$
type: string
mode:
description: 'The mode of operation of the Clowder Database
Provider. Valid options are: (*_app-interface_*) where
Expand Down Expand Up @@ -197,6 +204,9 @@ objects:
Kafka cluster is deleted Only applies when KafkaConfig.PVC
is set to 'true'
type: boolean
forceTLS:
description: Force TLS
type: boolean
jvmOptions:
description: JVM Options
properties:
Expand Down Expand Up @@ -322,25 +332,6 @@ objects:
description: EnableLegacyStrimzi disables TLS + user
auth
type: boolean
ephemManagedDeletePrefix:
description: Defines a prefix whitelist value
type: string
ephemManagedSecretRef:
description: Defines the secret reference for the Ephemeral
Managed Kafka mode. Only used in (*_managed-ephem_*)
mode.
properties:
name:
description: Name defines the Name of a resource.
type: string
namespace:
description: Namespace defines the Namespace of
a resource.
type: string
required:
- name
- namespace
type: object
managedPrefix:
description: Managed topic prefix for the managed cluster.
Only used in (*_managed_*) mode.
Expand Down Expand Up @@ -552,6 +543,30 @@ objects:
the default resource requirements from the ClowdEnvironment
will be used.
properties:
claims:
description: "Claims lists the names of resources,\
\ defined in spec.resourceClaims, that are\
\ used by this container. \n This is an alpha\
\ field and requires enabling the DynamicResourceAllocation\
\ feature gate. \n This field is immutable."
items:
description: ResourceClaim references one
entry in PodSpec.ResourceClaims.
properties:
name:
description: Name must match the name
of one entry in pod.spec.resourceClaims
of the Pod where this field is used.
It makes that resource available inside
a container.
type: string
required:
- name
type: object
type: array
x-kubernetes-list-map-keys:
- name
x-kubernetes-list-type: map
limits:
additionalProperties:
anyOf:
Expand Down Expand Up @@ -597,6 +612,31 @@ objects:
description: Defines the resource requests/limits
set on selenium containers
properties:
claims:
description: "Claims lists the names\
\ of resources, defined in spec.resourceClaims,\
\ that are used by this container.\
\ \n This is an alpha field and requires\
\ enabling the DynamicResourceAllocation\
\ feature gate. \n This field is immutable."
items:
description: ResourceClaim references
one entry in PodSpec.ResourceClaims.
properties:
name:
description: Name must match the
name of one entry in pod.spec.resourceClaims
of the Pod where this field
is used. It makes that resource
available inside a container.
type: string
required:
- name
type: object
type: array
x-kubernetes-list-map-keys:
- name
x-kubernetes-list-type: map
limits:
additionalProperties:
anyOf:
Expand Down Expand Up @@ -727,6 +767,18 @@ objects:
a ClowdApp should be served on.
format: int32
type: integer
tls:
description: TLS sidecar enablement
properties:
enabled:
type: boolean
port:
format: int32
type: integer
privatePort:
format: int32
type: integer
type: object
required:
- mode
- port
Expand All @@ -742,6 +794,28 @@ objects:
k8s format in the event that they omitted from a PodSpec inside
a ClowdApp.
properties:
claims:
description: "Claims lists the names of resources, defined\
\ in spec.resourceClaims, that are used by this container.\
\ \n This is an alpha field and requires enabling the\
\ DynamicResourceAllocation feature gate. \n This field\
\ is immutable."
items:
description: ResourceClaim references one entry in PodSpec.ResourceClaims.
properties:
name:
description: Name must match the name of one entry
in pod.spec.resourceClaims of the Pod where this
field is used. It makes that resource available
inside a container.
type: string
required:
- name
type: object
type: array
x-kubernetes-list-map-keys:
- name
x-kubernetes-list-type: map
limits:
additionalProperties:
anyOf:
Expand Down Expand Up @@ -1011,6 +1085,7 @@ objects:
type: object
type: array
type: object
x-kubernetes-map-type: atomic
scopes:
description: A collection of filters that must match
each object tracked by a quota. If not specified,
Expand Down Expand Up @@ -1134,17 +1209,11 @@ objects:
storage: true
subresources:
status: {}
status:
acceptedNames:
kind: ''
plural: ''
conditions: []
storedVersions: []
- apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: v0.8.0
controller-gen.kubebuilder.io/version: v0.11.1
creationTimestamp: null
name: namespacereservations.cloud.redhat.com
spec:
Expand Down Expand Up @@ -1239,12 +1308,6 @@ objects:
storage: true
subresources:
status: {}
status:
acceptedNames:
kind: ''
plural: ''
conditions: []
storedVersions: []
- apiVersion: v1
kind: ServiceAccount
metadata:
Expand Down

0 comments on commit 4dae07b

Please sign in to comment.