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

Sync with manual mapping not working when multiple files are changed of which the first one is not mapped #9326

Open
simonstratmann opened this issue Feb 26, 2024 · 2 comments · May be fixed by #9344
Open

Sync with manual mapping not working when multiple files are changed of which the first one is not mapped #9326

simonstratmann opened this issue Feb 26, 2024 · 2 comments · May be fixed by #9344

Comments

@simonstratmann
Copy link

Expected behavior

When two files are modified in one session and only the second is included in a manual sync mapping then a sync should be executed.

Actual behavior

When two files are modified in one session and only the second is included in a manual sync mapping then no sync is being executed. If only the second file is modified then sync is executed. In other words, for any number of changed files if the first in the list does not match the pattern then no sync is being executed.

Information

  • Skaffold version: 2.10.1
  • Operating system: Windows 10
  • Installed via: Google Cloud code
  • Contents of skaffold.yaml (excerpt):
sync:
    manual:
        - src: "project-rest/target/classes/**/*.class"
        dest: "/app/classes/"
        strip: "project-rest/target/classes/"

When .java and .class files are modified (by changing the java file in IntelliJ and compiling):

files modified: [project-rest\src\main\java\package\WebController.java project-rest\target\classes\package\WebController.class]
files modified: [project-rest\src\main\java\package\WebController.java project-rest\target\classes\package\WebController.class]
Changed file project-rest\src\main\java\package\WebController.java does not match any sync pattern. Skipping sync
dev intents: build false, sync true, deploy false

When only the .class file is modified ((by changing the java file in IntelliJ, changing the window for IntelliJ to write the java file and then compiling):

files modified: [project-rest\target\classes\package\WebController.class]
files modified: [.idea\workspace.xml project-rest\target\classes\package\WebController.class]
dev intents: build false, sync true, deploy false

 devloop: build false, sync true, deploy false

Syncing 1 files for localhost:5010/project-application:jbo_temp_20240220-9-gff455bb-dirty@sha256:0a3adc1d601c8097a7a4ea6665de500422afa7bb1807776a3e651a51fd8ee5c5

I'm working on a sample project to reproduce but I'm having issues with docker registry authentication.

This problem occurs with any number of modified files:

files modified: [project-application\target\classes\banner.txt project-application\target\classes\bootstrap.yml project-rest\target\classes\de\ppi\pcy\gen\rest\WebController.class]
files modified: [.idea\compiler.xml project-application\target\classes\banner.txt project-application\target\classes\bootstrap.yml project-common\target\test-classes\attachments\E2EID-1-dummy.pdf project-common\target\test-classes\attachments\E2EID-1-empty.pdf project-common\target\test-classes\attachments\E2EID-2-dummy.pdf project-common\target\test-classes\jsons\many-date-time-formats.json project-common\target\test-classes\jsons\many-with-errors.json project-common\target\test-classes\jsons\many.json project-common\target\test-classes\jsons\single-complete.json project-common\target\test-classes\jsons\single-minimal.json project-rest\src\main\java\de\ppi\pcy\gen\rest\WebController.java project-rest\target\classes\de\ppi\pcy\gen\rest\WebController.class]
Changed file project-application\target\classes\banner.txt does not match any sync pattern. Skipping sync

Only the first file is taken into account while any of the files matching the pattern should result in a sync.
@simonstratmann simonstratmann changed the title Manually triggered sync with manual mapping not working when multiple files are changed of which the first one is not mapped Sync with manual mapping not working when multiple files are changed of which the first one is not mapped Mar 7, 2024
simonstratmann added a commit to simonstratmann/skaffold that referenced this issue Mar 14, 2024
@simonstratmann
Reproduce manual match bug
96f5432 
See GoogleContainerTools#9326
@simonstratmann
Copy link
Author

I was able to reproduce this bug by making a simple change to the appropriate unit test: simonstratmann@96f5432

I don't know Go at all and can't begin to comprehend how to fix this bug, though. Perhaps I'm misunderstanding the idea behind manual sync but with a change event with multiple files why would each file have to match the sync patterns.

I think that pkg/skaffold/sync/sync.go:228 (intersect) should not return as soon as a file does not match. I tried replacing return nil, nil with continue which actually fixed my problem but made many of the other manual tests fail. I'm sure if you know what you're doing this is easy to fix.

simonstratmann added a commit to simonstratmann/skaffold that referenced this issue Mar 14, 2024
@simonstratmann
Dirty hack to fix sync bug
35816c0 
See GoogleContainerTools#9326
@simonstratmann
Copy link
Author

I was actually able to fix the bug and keep the other tests successful. I added a dirty hack for the latter because the expectations should probably be different.

simonstratmann added a commit to simonstratmann/skaffold that referenced this issue Mar 14, 2024
@simonstratmann
fix: Allow partial match for manual sync
dc25420 
See GoogleContainerTools#9326
simonstratmann added a commit to simonstratmann/skaffold that referenced this issue Mar 14, 2024
@simonstratmann
fix: Allow partial match for manual sync
eeafea6 
See GoogleContainerTools#9326
@simonstratmann simonstratmann linked a pull request Mar 14, 2024 that will close this issue
Open
simonstratmann added a commit to simonstratmann/skaffold that referenced this issue Mar 18, 2024
@simonstratmann
fix: Allow partial match for manual sync
b62745c 
See GoogleContainerTools#9326
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: Allow partial match for manual sync simonstratmann/skaffold
1 participant
@simonstratmann