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

vendor: update docker/docker master, and remove uses of github.com/moby/sys/user #5708

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

vendor: update docker/docker master, and remove uses of github.com/moby/sys/user #5708

wants to merge 1 commit into from
+13 −13

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Dec 21, 2024 •
edited
Loading

- What I did

- How I did it

- How to verify it

- Description for the changelog



- A picture of a cute animal (not mandatory but encouraged)

    

  
  



      


          

          
  

    
  
    
    

  

  




          

        
      




  





       
            



      

  
            


    

    

      
    

    


      

          @thaJeztah
thaJeztah




          added this to the   28.0.0 milestone


      Dec 21, 2024

    

  



    

    

      
    

    


      

          @thaJeztah
thaJeztah




              self-assigned this


      Dec 21, 2024

    

  








      

  
      



  

  @codecov-commenter





  


    

      

  

    

        
    
    

  


      
          
  
      
            Copy link

  

      
    

  


  

      



      

  


  

    

      

      
            codecov-commenter
  

      

      

      commented


        Dec 21, 2024


      
  •

  

    
      

        
          edited
          
        
        
      

    
    
      
        
      Loading

      
    
  



    


  





      


        


  
    

      
          
Codecov Report


All modified and coverable lines are covered by tests ✅




Project coverage is 59.51%. Comparing base (ff4697d) to head (43fed0c).




Additional details and impacted files

@@           Coverage Diff           @@
##           master    #5708   +/-   ##
=======================================
  Coverage   59.51%   59.51%           
=======================================
  Files         346      346           
  Lines       29376    29376           
=======================================
  Hits        17483    17483           
  Misses      10923    10923           
  Partials      970      970           



      		
    

  





        


            

            

              
            

        

      


      

            
  

    
  
    
    

  

  



    












      

  
          
    

    

      
    

    


      

          @thaJeztah
thaJeztah




        changed the title
vendor: update docker/docker master, and remove uses of pkg/system

vendor: update docker/docker master, and remove uses of github.com/moby/sys/user


      Dec 23, 2024

    

  



  





      

  
      



  

  @thaJeztah





  


    

      

  

    

        
    
    

  


      
          
  
      
            Copy link

  

      
    

  


  

      

  
  Member


      

  Author


  


  

    

  





      


        


  
    

      
          
After this, pkg/pools is still in use in the CLI for the Classic Builder; 

  

    

      cli/cli/command/image/build/context.go
    

    

        Lines 426 to 447
      in
      3b49deb
    

  

  

    


        

          
           // Compress the build context for sending to the API 
        


        

          
           func Compress(buildCtx io.ReadCloser) (io.ReadCloser, error) { 
        


        

          
           	pipeReader, pipeWriter := io.Pipe() 
        


        

          
            
        


        

          
           	go func() { 
        


        

          
           		compressWriter, err := archive.CompressStream(pipeWriter, archive.Gzip) 
        


        

          
           		if err != nil { 
        


        

          
           			pipeWriter.CloseWithError(err) 
        


        

          
           		} 
        


        

          
           		defer buildCtx.Close() 
        


        

          
            
        


        

          
           		if _, err := pools.Copy(compressWriter, buildCtx); err != nil { 
        


        

          
           			pipeWriter.CloseWithError(errors.Wrap(err, "failed to compress context")) 
        


        

          
           			compressWriter.Close() 
        


        

          
           			return 
        


        

          
           		} 
        


        

          
           		compressWriter.Close() 
        


        

          
           		pipeWriter.Close() 
        


        

          
           	}() 
        


        

          
            
        


        

          
           	return pipeReader, nil 
        


        

          
           } 
        

    

  









That was;



However, on that PR, it was suggested to move the build.Compress function to pkg/archive in moby; #233 (comment) - looks  like that never happend.

      		
    

  





        


            

            

              
            

        

      


      

            
  

    
  
    
    

  

  



    












      

  
      



  

  @thaJeztah





  


    

      

  

    

        
    
    

  


      
          
  
      
            Copy link

  

      
    

  


  

      

  
  Member


      

  Author


  


  

    

  





      


        


  
    

      
          
Failures look legit / related;


time="2024-12-25T10:56:00Z" level=error msg="Can't add file /var/folders/95/0ydz4d79163427j3k5crp3fh0000gn/T/cp-test-465366053/file1 to tar: open /var/folders/95/0ydz4d79163427j3k5crp3fh0000gn/T/cp-test-465366053/file1: no such file or directory"
time="2024-12-25T10:56:00Z" level=error msg="Can't close tar writer: archive/tar: missed writing 8 bytes"
--- FAIL: TestRunCopyFromContainerToFilesystem (0.01s)
    cp_test.go:85: assertion failed: error is not nil: unexpected EOF
Error: something went wrong
Error: exit status 33
Error: exit status 130
time="2024-12-25T10:56:01Z" level=error msg="Error waiting for container: removal failed"
time="2024-12-25T10:56:01Z" level=error msg="error waiting for container: no such container: non-existent-container-id"
FAIL


      		
    

  





        


            

            

              
            

        

      


      

            
  

    
  
    
    

  

  



    












      

  
          

  

    
  


  

      

    @thaJeztah
thaJeztah



    mentioned this pull request
    
      Dec 25, 2024
    





  


  

      
   Merged

  







  




  





      

  
      



  

  @thaJeztah





  


    

      

  

    

        
    
    

  


      
          
  
      
            Copy link

  

      
    

  


  

      

  
  Member


      

  Author


  


  

    

  





      


        


  
    

      
          
Rebased after #5715 was merged, which  should fix the faulty test.

      		
    

  





        


            

            

              
            

        

      


      

            
  

    
  
    
    

  

  



    












      

  
          

  
  

  
      @thaJeztah
  thaJeztah


        force-pushed
    the
        
      
  bump_engine


    
 branch
      2 times, most recently
    from
    c113896    to
    fb49050      
    Compare
  



    January 1, 2025 15:33    
    




  





      

  
        





      

  
          









  
  

    

      

  




    


    

      

              

  
    Sign up for free
    to join this conversation on GitHub.
    Already have an account?
    Sign in to comment


  



      

    

  




  
          




      

  

    
    

    
  

    Reviewers
  


    

    No reviews






    

  


      
  

    Assignees
  



        

      




        

  
      @thaJeztah
  
    thaJeztah
    

      








      


  


  

    Labels
  



    

      

    area/dependencies

  

    kind/refactor

  PR's that refactor, or clean-up code
  

    status/2-code-review









      

  

    

        

    Projects
  


        




    None yet




  



      


  

    
  

    Milestone
  


      
    
  
    28.0.0
  





    
      
          


  

    
      

        
    

    Development
  



          


Successfully merging this pull request may close these issues.



  


    
  




      

    

  
      

  

    

      2 participants
    

    

        
          @thaJeztah 
        
          @codecov-commenter