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.

Sign up for GitHub

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

Jump to bottom

Upgrade K8s API version and move Octant plugin to seperate folder #838

Merged
merged 1 commit into from
Jun 23, 2020
Merged

Upgrade K8s API version and move Octant plugin to seperate folder #838

merged 1 commit into from
Jun 23, 2020

Conversation

mengdie-song
Copy link
Contributor

@mengdie-song mengdie-song commented Jun 16, 2020 •
edited
Loading

To use latest version of octant and decouple K8s API
version dependency between Antrea and Octant, this patch
includes the following changes.

  1. Upgrade Antrea K8s API to 1.18 which already contains
    some broken API changesfor generated client like extra field of context.
    In this way, we can just use same client for both Antrea and plugins.
    There will be no duplicate code introduced.

  2. Move Octant plugins to a separate folder named plugins
    and make it a separate sub project.

  3. Use latest octant 0.13.1 which has fixes graphviz related issues,
    so that traceflow-plugin can use this later.

Fixes: #837, #525

@antrea-bot
Copy link
Collaborator

Thanks for your PR.
Unit tests and code linters are run automatically every time the PR is updated.
E2e, conformance and network policy tests can only be triggered by a member of the vmware-tanzu organization. Regular contributors to the project should join the org.

The following commands are available:

  • /test-e2e: to trigger e2e tests.
  • /skip-e2e: to skip e2e tests.
  • /test-conformance: to trigger conformance tests.
  • /skip-conformance: to skip conformance tests.
  • /test-networkpolicy: to trigger networkpolicy tests.
  • /skip-networkpolicy: to skip networkpolicy tests.
  • /test-windows-conformance: to trigger windows conformance tests.
  • /skip-windows-conformance: to skip windows conformance tests.
  • /test-all: to trigger all tests.
  • /skip-all: to skip all tests.

These commands can only be run by members of the vmware-tanzu organization.

@mengdie-song
Copy link
Contributor Author

@tnqn For this change, I temporarily remove the client-go replacement you have added previously in go.mod. Could you help provide another one with K8s API 1.18 after we decide to have this change?

antoninbas
antoninbas reviewed Jun 17, 2020
View reviewed changes
Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • I believe this PR will also close Support latest client-go updates #525?

  • Can we add an extra level of directory: plugins/ -> plugins/octant/

  • I think this approach (new Go module for the Octant plugin) is an improvement, even though the new module is kind of "second-class" since we are stuck with the github.com/vmware-tanzu/antrea => ../ replace statement unless we are very careful about updating the dependency on a correct Antrea version

  • Please remove the following sentence from your commit message, you have a single commit anyway :):

Note: 1 and 2 need to be in the same commit since 2 will reply on
the updated Antrea with K8s API 1.18

plugins/go.mod Outdated Show resolved Hide resolved
plugins/go.mod Outdated Show resolved Hide resolved
plugins/go.mod Outdated Show resolved Hide resolved
plugins/go.sum Outdated Show resolved Hide resolved
@mengdie-song
Copy link
Contributor Author

@antoninbas Thanks for the comments. I will update them in the next patch.
And with this patch, we can also close #525.

tnqn
tnqn reviewed Jun 17, 2020
View reviewed changes
build/images/codegen/Dockerfile Outdated Show resolved Hide resolved
pkg/apiserver/registry/system/controllerinfo/rest.go Outdated Show resolved Hide resolved
plugins/Makefile Outdated Show resolved Hide resolved
plugins/go.mod Outdated Show resolved Hide resolved
plugins/go.mod Outdated Show resolved Hide resolved
@tnqn
Copy link
Member

tnqn commented Jun 17, 2020

@tnqn For this change, I temporarily remove the client-go replacement you have added previously in go.mod. Could you help provide another one with K8s API 1.18 after we decide to have this change?

Sure, will do, but I suggest to base on latest 1.18 patch release.

@antrea-bot
Copy link
Collaborator

Thanks for your PR.
Unit tests and code linters are run automatically every time the PR is updated.
E2e, conformance and network policy tests can only be triggered by a member of the vmware-tanzu organization. Regular contributors to the project should join the org.

The following commands are available:

  • /test-e2e: to trigger e2e tests.
  • /skip-e2e: to skip e2e tests.
  • /test-conformance: to trigger conformance tests.
  • /skip-conformance: to skip conformance tests.
  • /test-whole-conformance: to trigger all conformance tests on linux.
  • /skip-whole-conformance: to skip all conformance tests on linux.
  • /test-networkpolicy: to trigger networkpolicy tests.
  • /skip-networkpolicy: to skip networkpolicy tests.
  • /test-windows-conformance: to trigger windows conformance tests.
  • /skip-windows-conformance: to skip windows conformance tests.
  • /test-all: to trigger all tests (except whole conformance).
  • /skip-all: to skip all tests (except whole conformance).

These commands can only be run by members of the vmware-tanzu organization.

@mengdie-song mengdie-song force-pushed the upgrade-1.18-mvfolder-rebase branch 2 times, most recently from ff8871f to d7030f5 Compare June 18, 2020 09:28
@tnqn
Copy link
Member

tnqn commented Jun 18, 2020

I think you need to fix api usages in test files.

tnqn
tnqn reviewed Jun 18, 2020
View reviewed changes
plugins/octant/Makefile Outdated Show resolved Hide resolved
plugins/octant/go.sum Show resolved Hide resolved
@mengdie-song mengdie-song force-pushed the upgrade-1.18-mvfolder-rebase branch 2 times, most recently from 31cfd12 to ae8105b Compare June 19, 2020 06:16
@mengdie-song
Copy link
Contributor Author

/test-all

@mengdie-song
Copy link
Contributor Author

/test-windows-conformance

1 similar comment
@mengdie-song
Copy link
Contributor Author

/test-windows-conformance

@mengdie-song
Copy link
Contributor Author

@tnqn @antoninbas I have updated tidy-check.sh, please help take a look. Confirmed with @weiqiangt, Makefile target docker-tidy seems to be useless, so I remove this target and we will only keep tidy and test-tidy.

@lzhecheng
Copy link
Contributor

/test-windows-conformance

tnqn
tnqn reviewed Jun 19, 2020
View reviewed changes
Makefile Outdated Show resolved Hide resolved
hack/tidy-check.sh Outdated Show resolved Hide resolved
@mengdie-song
Copy link
Contributor Author

/test-all

@mengdie-song
Copy link
Contributor Author

/test-windows-conformance

tnqn
tnqn reviewed Jun 19, 2020
View reviewed changes
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

hack/tidy-check.sh Show resolved Hide resolved
tnqn
tnqn previously approved these changes Jun 19, 2020
View reviewed changes
@jianjuns
Copy link
Contributor

I like the change to move Octant plugin to a separate project/dir.

antoninbas
antoninbas reviewed Jun 20, 2020
View reviewed changes
Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good to me, I just have questions about the changes to tidy-check.sh

Makefile Show resolved Hide resolved
hack/tidy-check.sh Show resolved Hide resolved
@mengdie-song
Copy link
Contributor Author

/test-all

@mengdie-song
Copy link
Contributor Author

/test-all

tnqn
tnqn reviewed Jun 22, 2020
View reviewed changes
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a nit-picking comment

hack/tidy-check.sh Show resolved Hide resolved
hack/tidy-check.sh Outdated Show resolved Hide resolved
@mengdie-song mengdie-song force-pushed the upgrade-1.18-mvfolder-rebase branch 2 times, most recently from 5bedb9c to 44a06e1 Compare June 22, 2020 10:10
@mengdie-song
Copy link
Contributor Author

/test-all

tnqn
tnqn previously approved these changes Jun 22, 2020
View reviewed changes
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

antoninbas
antoninbas previously approved these changes Jun 23, 2020
View reviewed changes
Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mengdie-song mengdie-song dismissed stale reviews from antoninbas and tnqn via 995fb18 June 23, 2020 01:22
@mengdie-song
Copy link
Contributor Author

Resolve merge conflict for test/e2e/security_test.go

@mengdie-song
Copy link
Contributor Author

/test-all

@mengdie-song
Copy link
Contributor Author

/test-all

@mengdie-song
Copy link
Contributor Author

/test-conformance

@mengdie-song
Copy link
Contributor Author

/test-all

1 similar comment
@lzhecheng
Copy link
Contributor

/test-all

@mengdie-song
Copy link
Contributor Author

/skip-all

@mengdie-song
Copy link
Contributor Author

Have confirmed that the latest Jenkins CI failures have nothing to do with this change. Since it has already passed Jenkins CI check previously and the latest update is just to address merge conflicts, merge this one.

@mengdie-song mengdie-song merged commit a054093 into antrea-io:master Jun 23, 2020
@mengdie-song
Upgrade K8s API version and move Octant plugin to seperate folder
98078b2 
To use latest version of octant and decouple K8s API
version dependency between Antrea and Octant, this patch
includes the following changes.

1. Upgrade Antrea K8s API to 1.18 which already contains
some broken API changesfor generated client like extra field of context.
In this way, we can just use same client for both Antrea and plugins.
There will be no duplicate code introduced.

2. Move Octant plugins to a separate folder named plugins
and make it a separate sub project.

3. Use latest octant 0.13.1 which has fixes graphviz related issues,
so that traceflow-plugin can use this later.

Fixes: antrea-io#837, antrea-io#525
weiqiangt added a commit to weiqiangt/antrea that referenced this pull request Jun 23, 2020
@weiqiangt
Revert "Upgrade K8s API version and move Octant plugin to seperate fo…
682c54f 
…lder (antrea-io#838)"

This reverts commit a054093

Signed-off-by: Weiqiang TANG <weiqiangt@vmware.com>
GraysonWu pushed a commit to GraysonWu/antrea that referenced this pull request Sep 22, 2020
@mengdie-song @GraysonWu
Upgrade K8s API version and move Octant plugin to seperate folder (an…
7b82798 
…trea-io#838)

To use latest version of octant and decouple K8s API
version dependency between Antrea and Octant, this patch
includes the following changes.

1. Upgrade Antrea K8s API to 1.18 which already contains
some broken API changesfor generated client like extra field of context.
In this way, we can just use same client for both Antrea and plugins.
There will be no duplicate code introduced.

2. Move Octant plugins to a separate folder named plugins
and make it a separate sub project.

3. Use latest octant 0.13.1 which has fixes graphviz related issues,
so that traceflow-plugin can use this later.

Fixes: antrea-io#837, antrea-io#525
GraysonWu pushed a commit to GraysonWu/antrea that referenced this pull request Sep 23, 2020
@mengdie-song @GraysonWu
Upgrade K8s API version and move Octant plugin to seperate folder (an…
c1ae9c3 
…trea-io#838)

To use latest version of octant and decouple K8s API
version dependency between Antrea and Octant, this patch
includes the following changes.

1. Upgrade Antrea K8s API to 1.18 which already contains
some broken API changesfor generated client like extra field of context.
In this way, we can just use same client for both Antrea and plugins.
There will be no duplicate code introduced.

2. Move Octant plugins to a separate folder named plugins
and make it a separate sub project.

3. Use latest octant 0.13.1 which has fixes graphviz related issues,
so that traceflow-plugin can use this later.

Fixes: antrea-io#837, antrea-io#525
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tnqn tnqn tnqn approved these changes

@antoninbas antoninbas antoninbas left review comments

@jianjuns jianjuns Awaiting requested review from jianjuns

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Upgrade Antrea K8s API version and Octant version to support Traceflow UI plugin
7 participants
@mengdie-song @antrea-bot @tnqn @lzhecheng @jianjuns @antoninbas @vmwclabot