-
Notifications
You must be signed in to change notification settings - Fork 1
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
chore: add a hash suffix based on the spec for pipeline #244
Conversation
hmm, looks like e2e failed? |
if pipelineSpec.InterStepBufferServiceName != "" { | ||
labelMapping[common.LabelKeyISBServiceNameForPipeline] = pipelineSpec.InterStepBufferServiceName | ||
} else { | ||
labelMapping[common.LabelKeyISBServiceNameForPipeline] = "default" |
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.
maybe doesn't need to be done in this PR, but are we going to add a label for PipelineRollout name? Then we can easily find all Pipelines using that PipelineRollout. I know the Pipeline name will be prefixed with the PipelineRollout name, but I think this would enable easy selection both through kubectl and potentially in the code.
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.
unless we can do a query on OwnerReference I guess...but just to note that Numaflow includes labels like this
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.
FYI, I have just created this issue which depends on this
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.
yeah, that will be the follow up PR for this.
Signed-off-by: Hao Hao <[email protected]>
b0cb33d
to
68d146a
Compare
Signed-off-by: Hao Hao <[email protected]>
68d146a
to
db6fc9d
Compare
Decided to postponed this PR as it can introduce the issue that multiple pipelines associate with a pipelineRollout after a spec change. |
Fixes #243
Modifications
Since we may have multiple pipelines associate with a pipeline rollout soon for the no downtime upgrade feature. This PR adds the logic to uniquely identify a pipeline that associate with a pipeline rollout based on its spec. To deterministic identify a pipeline, added a hash of the pipeline spec as a suffix to the name of the pipeline.
This PR also fixes a bug that when a pipeline with the same name already exist without a pipeline Rollout manage it. It should update the phase to failed without checking the child status.
Verification