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

Epic: submariner-addon #1517

Closed
3 of 9 tasks
skitt opened this issue Aug 4, 2021 · 14 comments
Closed
3 of 9 tasks

Epic: submariner-addon #1517

skitt opened this issue Aug 4, 2021 · 14 comments
Assignees
@dfarrell07 @skitt @Jaanki
Labels
dependent enhancement New feature or request open-cluster-management Of interest to open-cluster-management/submariner-addon priority:high

Comments

@skitt
Copy link
Member

skitt commented Aug 4, 2021 •
edited by tpantelis
Loading

This epic tracks work to be done on https://github.com/open-cluster-management/submariner-addon:

  1. Repo infrastructure and mechanics
  2. Testing: add a job in Submariner to test with OCM (smattar)
  3. Deprecation of SubmarinerConfig in favor of https://github.com/submariner-io/cloud-prepare (skitt)
  4. Integration logic itself - can we offload some of it to the Submariner Operator? Depends on Move resources creation logic to submariner-operator stolostron/submariner-addon#123 (Janki)
  5. Feature set/UX (e.g features which are available in Submariner/subctl but are not exposed in the addon side)
  6. Make sure the documentation is updated with any changes made above
  7. Document how to troubleshoot Submariner with OCM (e.g. how to run subctl gather)
@skitt skitt added enhancement New feature or request open-cluster-management Of interest to open-cluster-management/submariner-addon labels Aug 4, 2021
@skitt skitt self-assigned this Aug 4, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Aug 4, 2021 •
edited
Loading

⏳ Alright! Looks like we need to wait for some dependencies:

💡 Don't worry, I will continue watching the list above and keep this comment updated. To add or remove a dependency please update this issue/PR description.

Brought to you by Dependent Issues (:robot: ). Happy coding!

@skitt
Copy link
Member Author

skitt commented Aug 4, 2021

@dfarrell07 could you expand the “linters and bots” section?

@nyechiel
Copy link
Member

nyechiel commented Aug 4, 2021

@skitt - we need to review the feature-set section. One thing that's missing in the list is VXLAN support, but there could be others.

@dfarrell07
Copy link
Member

Determine whether we want to use Shipyard

I think this is one of the first things we need to do, as how we want to add linting depends on what we decide.

I tend to think we should use Shipyard's infra.

@dfarrell07
Copy link
Member

Continuing to think we should build on Shipyard. I'm struggling, getting a taste of how it would be without Shipyard, with duplicating the infra required to add basic gitlint CI.

The Makefile here as-is seems to require Go 1.15 for everything. The latest available from Ubuntu's package manager is 1.13, which why I'm having to install it manually.

dfarrell07/submariner-addon#1

@skitt
Copy link
Member Author

skitt commented Aug 5, 2021

Continuing to think we should build on Shipyard. I'm struggling, getting a taste of how it would be without Shipyard, with duplicating the infra required to add basic gitlint CI.

Right, that’s what I’m thinking too. My main concern is how well our Dapper-based builds will work with Prow; I reckon the best way to find that out would be to open a PR using Dapper ;-).

The Makefile here as-is seems to require Go 1.15 for everything. The latest available from Ubuntu's package manager is 1.13, which why I'm having to install it manually.

We even need 1.16 for the build itself (see go.mod); Ubuntu 21.04 has everything required, as does Fedora 34 of course, but some mainstream distributions don’t (notably, Debian 11 will have Go 1.15, and RHEL 8.4 has 1.15).

@skitt
Copy link
Member Author

skitt commented Aug 5, 2021

@skitt - we need to review the feature-set section. One thing that's missing in the list is VXLAN support, but there could be others.

I’ve added the missing pieces from the feature matrix (look at the last edit).

@dfarrell07
Copy link
Member

dfarrell07 commented Aug 5, 2021 •
edited
Loading

The Makefile here as-is seems to require Go 1.15 for everything. The latest available from Ubuntu's package manager is 1.13, which why I'm having to install it manually.

We even need 1.16 for the build itself (see go.mod); Ubuntu 21.04 has everything required, as does Fedora 34 of course, but some mainstream distributions don’t (notably, Debian 11 will have Go 1.15, and RHEL 8.4 has 1.15).

I think we have Ubuntu 20.04 on the ubuntu-latest GHA runner.

https://github.com/actions/virtual-environments#available-environments

It seems to install 1.13.

go version go1.13.8 linux/amd64

https://github.com/dfarrell07/submariner-addon/pull/2/checks?check_run_id=3253760648

My main concern is how well our Dapper-based builds will work with Prow; I reckon the best way to find that out would be to open a PR using Dapper ;-)

I made a basic Shipyard Makefile/Dockerfile.dapper per the docs, which was going fine until I realized I had clobbered the existing Makefile. 😅

On a somewhat-related note, I just saved your custom-Dapper PR from stalebot. Has Mike's feedback been addressed? Can we merge it? submariner-io/shipyard#594

@skitt
Copy link
Member Author

skitt commented Aug 6, 2021

The Makefile here as-is seems to require Go 1.15 for everything. The latest available from Ubuntu's package manager is 1.13, which why I'm having to install it manually.

We even need 1.16 for the build itself (see go.mod); Ubuntu 21.04 has everything required, as does Fedora 34 of course, but some mainstream distributions don’t (notably, Debian 11 will have Go 1.15, and RHEL 8.4 has 1.15).

I think we have Ubuntu 20.04 on the ubuntu-latest GHA runner.

https://github.com/actions/virtual-environments#available-environments

Yes, runners only provide LTS releases of Ubuntu; the next update will be 22.04.

On a somewhat-related note, I just saved your custom-Dapper PR from stalebot. Has Mike's feedback been addressed? Can we merge it? submariner-io/shipyard#594

I think it has, but I’d rather wait for Mike to take another look at it — there’s no rush. (And thanks for re-opening it!)

@skitt
Copy link
Member Author

skitt commented Aug 11, 2021

@SteveMattar would you be interested in working on adding an E2E test using OCM in Submariner? It should be similar to the work you’ve done to test the bundle with OLM. There’s already a “deploy” target in the addon which sets up kind with OLM and OCM.

@SteveMattar
Copy link

@skitt yes sure I will.

@dfarrell07 dfarrell07 self-assigned this Aug 17, 2021
@dfarrell07
Copy link
Member

dfarrell07 commented Aug 24, 2021 •
edited
Loading

We talked about moving away from vendoring today on the addon sync. In summary, it doesn't seem like vendoring is worth it. In particular:

  • Assurance of artifacts being what we expect can be provided by go.sum.
  • Caching for temporary outages can be provided by goproxy.
  • If a dependency were to really disappear (be deleted), we'd want to take action to move away from it anyway.

The costs to vendoring are substantial:

  • ~100K LoC diffs when dependencies change.
  • Git repo histoires include everything ever vendored, making the repo very large.
    • The k8s/k8s repo is 775.59 MiB.
    • The k8s/test-infra repo is 164.56 MiB.

@Jaanki Jaanki self-assigned this Sep 9, 2021
@ruromero ruromero mentioned this issue Oct 27, 2021
Merged
@maayanf24
Copy link
Contributor

@skitt Can we close this epic and track remaining work in new epic / issues?

@skitt
Copy link
Member Author

skitt commented Nov 2, 2021

Yes, let’s close this, it has run its course. Remaining tasks should be added to appropriate epics or handled as individual issues.

@skitt skitt closed this as completed Nov 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dfarrell07 dfarrell07

@skitt skitt

@Jaanki Jaanki

Labels
dependent enhancement New feature or request open-cluster-management Of interest to open-cluster-management/submariner-addon priority:high
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@dfarrell07 @skitt @nyechiel @Jaanki @SteveMattar @maayanf24