Skip to content

Commit 209d783

Browse files
committed
updating warning message to be more descriptive, add warning event on installplan
Signed-off-by: everettraven <everettraven@gmail.com>
1 parent 81d1ed4 commit 209d783

File tree

2 files changed

+19
-9
lines changed

2 files changed

+19
-9
lines changed

pkg/controller/operators/catalog/operator.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -2322,7 +2322,7 @@ func (o *Operator) ExecutePlan(plan *v1alpha1.InstallPlan) error {
23222322
o.logger.Errorf("failed to get a client for plan execution- %v", err)
23232323
return err
23242324
}
2325-
b := newBuilder(plan, o.lister.OperatorsV1alpha1().ClusterServiceVersionLister(), builderKubeClient, builderDynamicClient, r, o.logger)
2325+
b := newBuilder(plan, o.lister.OperatorsV1alpha1().ClusterServiceVersionLister(), builderKubeClient, builderDynamicClient, r, o.logger, o.recorder)
23262326

23272327
for i, step := range plan.Status.Plan {
23282328
if err := func(i int, step *v1alpha1.Step) error {

pkg/controller/operators/catalog/step.go

+18-8
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,15 @@ import (
77
"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/install"
88
"github.com/pkg/errors"
99
"github.com/sirupsen/logrus"
10+
corev1 "k8s.io/api/core/v1"
1011
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
1112
apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
1213
apiextensionsv1client "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1"
1314
apiextensionsv1beta1client "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1beta1"
1415
apierrors "k8s.io/apimachinery/pkg/api/errors"
1516
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1617
"k8s.io/client-go/dynamic"
18+
"k8s.io/client-go/tools/record"
1719
"k8s.io/client-go/util/retry"
1820

1921
"github.com/operator-framework/api/pkg/operators/v1alpha1"
@@ -44,18 +46,20 @@ type builder struct {
4446
dynamicClient dynamic.Interface
4547
manifestResolver ManifestResolver
4648
logger logrus.FieldLogger
49+
eventRecorder record.EventRecorder
4750

4851
annotator alongside.Annotator
4952
}
5053

51-
func newBuilder(plan *v1alpha1.InstallPlan, csvLister listersv1alpha1.ClusterServiceVersionLister, opclient operatorclient.ClientInterface, dynamicClient dynamic.Interface, manifestResolver ManifestResolver, logger logrus.FieldLogger) *builder {
54+
func newBuilder(plan *v1alpha1.InstallPlan, csvLister listersv1alpha1.ClusterServiceVersionLister, opclient operatorclient.ClientInterface, dynamicClient dynamic.Interface, manifestResolver ManifestResolver, logger logrus.FieldLogger, er record.EventRecorder) *builder {
5255
return &builder{
5356
plan: plan,
5457
csvLister: csvLister,
5558
opclient: opclient,
5659
dynamicClient: dynamicClient,
5760
manifestResolver: manifestResolver,
5861
logger: logger,
62+
eventRecorder: er,
5963
}
6064
}
6165

@@ -141,20 +145,26 @@ func (b *builder) NewCRDV1Step(client apiextensionsv1client.ApiextensionsV1Inter
141145
crd.SetResourceVersion(currentCRD.GetResourceVersion())
142146
if err = validateV1CRDCompatibility(b.dynamicClient, currentCRD, crd); err != nil {
143147
vErr := &validationError{}
144-
// if the conversion strategy in the new CRD is "None" OR the error is not a ValidationError
148+
// if the conversion strategy in the new CRD is not "Webhook" OR the error is not a ValidationError
145149
// return an error. This will catch and return any errors that occur unrelated to actual validation.
146150
// For example, the API server returning an error when performing a list operation
147-
if crd.Spec.Conversion == nil || crd.Spec.Conversion.Strategy == apiextensionsv1.NoneConverter || !errors.As(err, vErr) {
151+
if crd.Spec.Conversion == nil || crd.Spec.Conversion.Strategy != apiextensionsv1.WebhookConverter || !errors.As(err, vErr) {
148152
return fmt.Errorf("error validating existing CRs against new CRD's schema for %q: %w", step.Resource.Name, err)
149153
}
150-
// If the conversion strategy in the new CRD is not "None" and the error that occurred
154+
// If the conversion strategy in the new CRD is "Webhook" and the error that occurred
151155
// is an error related to validation, warn that validation failed but that we are trusting
152156
// that the conversion strategy specified by the author will successfully convert to a format
153157
// that passes validation and allow the upgrade to continue
154-
b.logger.Warnf("trusting conversion strategy detected in new CRD, but failed validation of existing CRs against new CRD's schema for %q: %s",
155-
step.Resource.Name,
156-
err.Error(),
157-
)
158+
warnTempl := `validation of existing CRs against new CRD's schema failed, but a webhook conversion strategy was specified.
159+
allowing the CRD upgrade to occur as we can't run the webhook, but is assumed that it will successfully convert existing CRs
160+
to a format that would have passed validation.
161+
162+
CRD: %q
163+
Validation Error: %s
164+
`
165+
warnString := fmt.Sprintf(warnTempl, step.Resource.Name, err.Error())
166+
b.logger.Warn(warnString)
167+
b.eventRecorder.Event(b.plan, corev1.EventTypeWarning, "CRDValidation", warnString)
158168
}
159169

160170
// check to see if stored versions changed and whether the upgrade could cause potential data loss

0 commit comments

Comments
 (0)