PROD-343: Change when the divert deploy logic is called to avoid problems when endpoints are defined in the manifest #4701
Conversation
This file contains hidden or 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
Signed-off-by: Nacho Fuertes <nacho@okteto.com>
ifbyol
commented
Apr 11, 2025
Comment on lines
-338
to
-346
| if params.Deployable.Divert != nil && params.Deployable.Divert.Namespace != params.Namespace { | ||
| oktetoLog.SetStage("Deploy Divert") | ||
| if err := r.DivertDeployer.Deploy(ctx); err != nil { | ||
| oktetoLog.AddToBuffer(oktetoLog.ErrorLevel, "error creating divert: %s", err.Error()) | ||
| return err | ||
| } | ||
| oktetoLog.SetStage("") | ||
| } | ||
|
|
There was a problem hiding this comment.
The change is to move this from there to cmd/deploy/deploy.go
Codecov ReportAttention: Patch coverage is
❌ Your patch status has failed because the patch coverage (40.74%) is below the target coverage (60.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #4701 +/- ##
==========================================
- Coverage 48.86% 48.84% -0.02%
==========================================
Files 354 354
Lines 29510 29517 +7
==========================================
- Hits 14421 14419 -2
- Misses 13945 13952 +7
- Partials 1144 1146 +2 🚀 New features to boost your workflow:
|
8000
Signed-off-by: Nacho Fuertes <nacho@okteto.com>
jLopezbarb
approved these changes
Apr 15, 2025
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.
Proposed changes
PROD-343
If you had a manifest with Divert, and then, you specify also the endpoints section, the update of the ingresses when the endpoints are deployed was making that the divert annotations were missing from the ingresses, so the header wasn't being propagated.
So, this started to happen when we re-organized the remote vs local deploy, as the divert deploy started to happen first. What I'm doing in this PR is to change the order, so, in the deployable phase (the one that can be happened locally or remotely, and the one running the proxy), we only apply the divert logic related to the proxy, but we don't call the
Deployfunction of the driver. That is started to be called at the end of the main flow (things that always happen locally), as the last stage of the deploy, so we ensure that the compose deployment (we already had issues in the past), and the endpoints deployment don't affect the divert logic.I have been checking, and there is no need of executing the divert deploy in the proxy.
I was testing all the output from the UI and CLI, and the output looks good too.
How to validate
This repository can be used to validate the changes: https://github.com/ifbyol/test-okteto-divert
Follow the next steps:
.envfile to et the namespace where you deployed the whole applicationokteto.serviceb.yamlas manifest in a new namespaceIf you access to the endpoint
https://test-okteto-divert-{rest-of-domain}/servicea/chain, you will see how three services are reported from the common namespace (which is wrong), and if you check the ingress generated in the last namespace, you will see how the divert annotations are missing. Also, you can see how the divert stage is executed before the "deploy compose"Now, build the CLI from this branch and follow the same steps described above (recommendation is to deploy it from scratch).
In this case, you will see that the endpoint works fine. If you check the ingress, you will see how the divert annotation is applied, and you can see how the divert stage is running after the "deploy compose" one. So, any endpoint generated from the deploy compose phase is also updated accordingly when the divert phase is executed