Skip to content

add networkmode service:#1848

Merged
k8s-ci-robot merged 5 commits intokubernetes:mainfrom
sosan:feature-1846-network_mode-service
Apr 25, 2024
Merged

add networkmode service:#1848
k8s-ci-robot merged 5 commits intokubernetes:mainfrom
sosan:feature-1846-network_mode-service

Conversation

@sosan
Copy link
Copy Markdown
Contributor

@sosan sosan commented Mar 30, 2024

What type of PR is this?

/kind feature

What this PR does / why we need it:

adjusting the network mode of services in docker compose (services:) and generate a mapping of deployments based on the network mode of each service merging containers into the destination deployment, and removing transferred deployments

compose.yaml example:

version: '3'

services:
  client:
    image: busybox
    command: ["sleep", "infinity"]
    container_name: threats-client
    restart: unless-stopped
    ports:
      - '8080:8080'

  server:
    image: busybox
    command: ["sleep", "infinity"]
    container_name: threats-server
    restart: unless-stopped
    network_mode: service:client

converted to add two container inside client-deployment:

....
 spec:
      containers:
        - args:
            - sleep
            - infinity
          image: busybox
          name: threats-client
          ports:
            - containerPort: 8080
              hostPort: 8080
              protocol: TCP
        - args:
            - sleep
            - infinity
          image: busybox
          name: threats-server
      restartPolicy: Always

Which issue(s) this PR fixes:

Add feature commented in #1846

Special notes for your reviewer:

@sosan
add networkmode service:
5897409 
added tests
fixNetworkModeToService is responsible for adjusting the network mode of services in docker compose (services:) and generate a mapping of deployments based on the network mode of each service merging containers into the destination deployment, and removing transferred deployments

Signed-off-by: jose luis <2064537+sosan@users.noreply.github.com>
@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Mar 30, 2024
@k8s-ci-robot k8s-ci-robot requested review from cdrage and kadel March 30, 2024 20:31
@k8s-ci-robot k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Mar 30, 2024
cdrage
cdrage requested changes Apr 2, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@cdrage cdrage left a comment

Choose a reason for hiding this comment

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

needs e2e tests / more tests, sorry!

Otherwise, this LGTM! I like the fact that you can add: network_mode: service:client and it'll group them in the same deployment. Great work.

@sosan
added tests
04acbf1 
add e2e
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Apr 3, 2024
@sosan
Copy link
Copy Markdown
Contributor Author

sosan commented Apr 3, 2024

added e2e and more tests

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 8, 2024
@sosan
Merge branch 'main' into feature-1846-network_mode-service
d43e210 
Signed-off-by: jose luis <2064537+sosan@users.noreply.github.com>
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 11, 2024
cdrage
cdrage reviewed Apr 19, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@cdrage cdrage left a comment •
edited
Loading

Choose a reason for hiding this comment

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

/lgtm

@cdrage
Copy link
Copy Markdown
Member

cdrage commented Apr 19, 2024

/approve

@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Apr 19, 2024
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 22, 2024
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

k8s-ci-robot commented Apr 25, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cdrage, sosan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@cdrage
Copy link
Copy Markdown
Member

cdrage commented Apr 25, 2024

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 25, 2024
@cdrage
Copy link
Copy Markdown
Member

cdrage commented Apr 25, 2024

@sosan looks like a failed test sorry, need it updated again :( I merged too many other PR's haha

@sosan
Merge branch 'main' into feature-1846-network_mode-service
3c189aa 
Signed-off-by: jose luis <2064537+sosan@users.noreply.github.com>
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 25, 2024
@sosan
Copy link
Copy Markdown
Contributor Author

sosan commented Apr 25, 2024 •
edited
Loading

@sosan looks like a failed test sorry, need it updated again :( I merged too many other PR's haha

no problem. merged

@cdrage
Copy link
Copy Markdown
Member

cdrage commented Apr 25, 2024

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 25, 2024
@k8s-ci-robot k8s-ci-robot merged commit e4ca58b into kubernetes:main Apr 25, 2024
@BrewTestBot BrewTestBot mentioned this pull request Apr 26, 2024
Merged
@devinrhode2 devinrhode2 mentioned this pull request Jun 3, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cdrage cdrage cdrage approved these changes

@kadel kadel Awaiting requested review from kadel

Assignees

@cdrage cdrage

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sosan @cdrage @k8s-ci-robot