Add manual approval component in Operator #2138
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tekton-robot
added
do-not-merge/work-in-progress
|
/test pull-tekton-operator-unit-tests |
PuneetPunamiya
force-pushed
the
add-manual-approval
branch
from
d0198a3
to
6035bec
Compare
|
The following is the coverage report on the affected files.
|
PuneetPunamiya
force-pushed
the
add-manual-approval
branch
2 times, most recently
from
8176b62
to
160e88a
Compare
|
The following is the coverage report on the affected files.
|
PuneetPunamiya
force-pushed
the
add-manual-approval
branch
from
160e88a
to
4bfb43c
Compare
|
The following is the coverage report on the affected files.
|
PuneetPunamiya
changed the title
tekton-robot
removed
the
do-not-merge/work-in-progress
|
/test pull-tekton-operator-build-tests |
piyush-garg
reviewed
May 7, 2024
piyush-garg
reviewed
May 7, 2024
piyush-garg
reviewed
May 7, 2024
piyush-garg
reviewed
May 7, 2024
piyush-garg
reviewed
May 7, 2024
piyush-garg
reviewed
May 7, 2024
piyush-garg
reviewed
May 7, 2024
| magCR := comp.(*v1alpha1.ManualApprovalGate) | ||
| magImages := common.ToLowerCaseKeys(common.ImagesFromEnv(common.ManualApprovalGatePrefix)) | ||
| extra := []mf.Transformer{ | ||
| common.InjectOperandNameLabelOverwriteExisting(v1alpha1.ManualApprovalGates), |
There was a problem hiding this comment.
dont we need to perform namespace change
piyush-garg
reviewed
May 7, 2024
| } | ||
|
|
||
| func (oe openshiftExtension) Transformers(comp v1alpha1.TektonComponent) []mf.Transformer { | ||
| return []mf.Transformer{} |
There was a problem hiding this comment.
no transformer required for openshift?
piyush-garg
reviewed
May 7, 2024
There was a problem hiding this comment.
Few points
- No documentation
- No enough unit tests here
pkg/reconciler/kubernetes/manualapprovalgate/transform.go - The code looks like manual approval will be deployed by default should we add under a flag to enable or disable like PAC, chains ??
- Can you update PR Description and also add a release notes ?
jkandasa
reviewed
May 8, 2024
There was a problem hiding this comment.
@PuneetPunamiya Thanks for the great PR 🎉
I have few questions
- How the namespace creation managed with
ManualtApprovalGate? ontekton-pipelinesnamespace and with custom namespace? - You need to add
ManualApprovalGatein toRemoveObsoleteSetsKind switch to remove the existing installersets (If there is a need)
khrm
reviewed
May 8, 2024
|
The following is the coverage report on the affected files.
|
PuneetPunamiya
force-pushed
the
add-manual-approval
branch
from
ea9e5fb
to
423a439
Compare
|
The following is the coverage report on the affected files.
|
piyush-garg
reviewed
May 13, 2024
piyush-garg
reviewed
May 13, 2024
PuneetPunamiya
force-pushed
the
add-manual-approval
branch
2 times, most recently
from
353ce55
to
e9ccb3b
Compare
|
The following is the coverage report on the affected files.
|
PuneetPunamiya
force-pushed
the
add-manual-approval
branch
2 times, most recently
from
341fd73
to
4fb9d1f
Compare
|
/approve |
tekton-robot
added
the
approved
|
Can you please add doc in a follow up PR |
|
The following is the coverage report on the affected files.
|
Signed-off-by: PuneetPunamiya <ppunamiy@redhat.com>
PuneetPunamiya
force-pushed
the
add-manual-approval
branch
from
4fb9d1f
to
1775859
Compare
Signed-off-by: PuneetPunamiya <ppunamiy@redhat.com>
Signed-off-by: PuneetPunamiya <ppunamiy@redhat.com>
|
The following is the coverage report on the affected files.
|
PuneetPunamiya
force-pushed
the
add-manual-approval
branch
from
1775859
to
eee5f23
Compare
|
The following is the coverage report on the affected files.
|
|
/test pull-tekton-operator-build-tests |
jkandasa
approved these changes
May 13, 2024
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jkandasa, piyush-garg The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm |
|
/lgtm |
jkandasa
added
the
kind/feature
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
make test lintbefore submitting a PRSee the contribution guide for more details.
Release Notes