Skip to content
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

feat: New yaml file for updated CNI and CNS version #2772

Merged
merged 2 commits into from
Jun 12, 2024

Conversation

chengch801214
Copy link
Contributor

Reason for Change:
Need an update to CNI version to resolve issue with newer kernels

Issue Fixed:

Requirements:

Notes:

@chengch801214 chengch801214 requested a review from a team as a code owner June 7, 2024 19:00
@chengch801214 chengch801214 requested a review from rayaisaiah June 7, 2024 19:00
tamilmani1989
tamilmani1989 previously approved these changes Jun 7, 2024
@chengch801214 chengch801214 removed the request for review from rayaisaiah June 7, 2024 23:31
paulyufan2
paulyufan2 previously approved these changes Jun 10, 2024
rbtr
rbtr previously requested changes Jun 10, 2024
@rbtr
Copy link
Contributor

rbtr commented Jun 10, 2024

Are the other manifests in this directory still in use? Is this strictly a version bump? Can we just write a kustomize template and let the consumer set these versions instead of hosting a manifest copy for every release?

@chengch801214
Copy link
Contributor Author

Are the other manifests in this directory still in use? Is this strictly a version bump? Can we just write a kustomize template and let the consumer set these versions instead of hosting a manifest copy for every release?

Yes the others are still in use. Two of them are the ones we communicated to OpenAI to use, but our tests no longer work with them, so we created two other ones with updated CNI version for two different tests.

@chengch801214 chengch801214 dismissed stale reviews from paulyufan2 and tamilmani1989 via 04cb495 June 10, 2024 23:59
@chengch801214
Copy link
Contributor Author

/azp run Azure Container Networking PR

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rbtr rbtr dismissed their stale review June 11, 2024 18:25

feedback addressed

@thomasricci
Copy link
Contributor

thomasricci commented Jun 11, 2024

Are the other manifests in this directory still in use? Is this strictly a version bump? Can we just write a kustomize template and let the consumer set these versions instead of hosting a manifest copy for every release?

yeah I think this makes sense. Otherwise we keep creating new files just to bump up versions. These files are purely consumer by our e2e pipeline and nobody else should be using them.

@chengch801214
Copy link
Contributor Author

Are the other manifests in this directory still in use? Is this strictly a version bump? Can we just write a kustomize template and let the consumer set these versions instead of hosting a manifest copy for every release?

yeah I think this makes sense. Otherwise we keep creating new files just to bump up versions. These files are purely consumer by our e2e pipeline and nobody else should be using them.

I'm going to merge this PR first to unblock our E2E test. Having a customizable way to input the version is useful and can be done next time we need an update.

@chengch801214 chengch801214 added this pull request to the merge queue Jun 11, 2024
@chengch801214 chengch801214 removed this pull request from the merge queue due to a manual request Jun 12, 2024
@chengch801214 chengch801214 added this pull request to the merge queue Jun 12, 2024
Merged via the queue into master with commit 7de898b Jun 12, 2024
11 checks passed
@chengch801214 chengch801214 deleted the chengch/mdnc-cnicns-update branch June 12, 2024 03:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants