-
Notifications
You must be signed in to change notification settings - Fork 545
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add installPlanApproval to Subscription-v1 #348
Add installPlanApproval to Subscription-v1 #348
Conversation
|
||
// GetInstallPlanApproval gets the configured install plan approval or the default | ||
func (s *Subscription) GetInstallPlanApproval() v1alpha1.Approval { | ||
if s.Spec.InstallPlanApproval == v1alpha1.ApprovalManual { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this works now because there are only 2 possible values, but it should really be:
if s.Spec.InstallPlanApproval == nil {
return v1alpha1.ApprovalAutomatic
}
return s.Spec.InstallPlanApproval
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did it this way so that any invalid approvals (approval: foo
etc) will get defaulted to Automatic
. If we ever get to more than two approval types I would change this to set logic (if approval not in set_of_allowed_values, return default)
965605e
to
455f15c
Compare
Setting installPlanApproval on the subscription directs OLM to create installPlans with the specified approval mode (Automatic or Manual)
ada40fb
to
62b2ef4
Compare
62b2ef4
to
51fd078
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, can't wait to add this to the UI!
83616a7
to
fea0ca3
Compare
fea0ca3
to
0105bac
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still LGTM, can't wait to add this to the UI!
…approvalmode Add installPlanApproval to Subscription-v1
…approvalmode Add installPlanApproval to Subscription-v1
The
installPlanApproval
field on Subscription will dictate theapproval
field of the InstallPlans that get created by the Subscription.Also removed the
Upgrade-Only
enum option, until we decide that it's valuable and implement it.